Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize control files creation and fixes files being truncated on some SMB file servers. #129

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tiarossi
Copy link

This PR aims to fix the situation occourred when creating a BagIt package inside a SMB file system. By some strange situation Files.write is ignoring the StandardOpenOption.APPEND option and truncating files on these repositories.

Please ensure you have completed the following before submitting:

  • Ran all tests to ensure existing functionality wasn't broken
  • Ran all quality assurance checks and fixed any new errors or warnings, which include:

Note: you can complete both boxes by running and fixing warnings/errors with gradle clean check

  • Code is self documenting or a short comment when self documenting isn't possible

@jscancella
Copy link
Contributor

Great find! We should add a unit test that only runs on SMB to guard against this in the future. Have you filed a bug report with Oracle/Java? Once they have this as a bug, we should do an inline comment citing the bug so that future developers understand why we aren't using Files.write

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 98.285% when pulling 3e7242c on silegis-mg:master into 2b7002e on LibraryOfCongress:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 98.285% when pulling 3e7242c on silegis-mg:master into 2b7002e on LibraryOfCongress:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 98.285% when pulling 3e7242c on silegis-mg:master into 2b7002e on LibraryOfCongress:master.

@tiarossi
Copy link
Author

tiarossi commented Nov 13, 2018

@jscancella, as i'm not sure if it is a Java bug, a Windows driver bug or a bug in the smb server we have been using, i didn't opened any bug report. Despite this, the change is a performance improvement as Files.write opens and closes the file at each access.

@jscancella
Copy link
Contributor

I know it is easier to not figure out where the bug is coming from, but without full knowledge we won't understand how much of an impact this will have to the whole userbase(is it only windows 2000 SP1 using an old version of SMB, or all windows for example?). I am also personally against writing code to fix someone's bug without reporting the bug so it can be fixed because it shifts the maintenance burden onto me.

I stand by Jack Diderich:

I hate code, and I want as little of it as possible in our product

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants