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

FileReplacer exception handling and tmpfile cleanup #44

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

Conversation

jdillon
Copy link
Contributor

@jdillon jdillon commented Jan 9, 2016

WIP, this does not yet pass tests due to one test explicitly assuming the tmpfile should remain on failure. Pending some use-case why this is good or desired.

// setup buffering, as almost certainly anywhere using this class is going to want this
BufferedOutputStream output = new BufferedOutputStream(new FileOutputStream(tempFile));
// prepare directory structure
file.getParentFile().mkdirs();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also should be verified to ensure sanity

@cstamas
Copy link
Contributor

cstamas commented Jan 20, 2016

AFAIR, the reason in NX2 to leave tmp file in place was for easier recovery. Did not find any direct proof of this decision, but these are IMHO related issues:
https://issues.sonatype.org/browse/NEXUS-5584
https://issues.sonatype.org/browse/NXCM-5302
https://issues.sonatype.org/browse/NXCM-4371

AFAIR, NX2 had a problem to kill some config files (staging.xml in this case), leaving 0 byte file in place, making it impossible to recover (with or without human intervention). Hence, we opted to leave tmp files as is to at least make human intervention possible/easier.

@jdillon
Copy link
Contributor Author

jdillon commented Jan 20, 2016

@cstamas isn't that why we built the replacer, so that the new file wouldn't corrupt the old file. If we failed to write the new file, or failed to replace the new file, wouldn't the internal state be corrupted anyways?

@cstamas
Copy link
Contributor

cstamas commented Jan 21, 2016

@jdillon yup, that is what I remember too.

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.

2 participants