Move in progress detection to separate file with pid #145
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This changes the logic of detecting if a backup is in progress by
writing out the snapshot and PID of the process making the backup. This
improves the situation where a previous backup was killed in a non-clean
way. Clean failures in the sense of a proper exit (also non 0 exit
statuses) or using ctrl-c was already handled by traps.
This just removes the in-progress file if the process is no longer
running after the backup. It then cleans up the in-progress file and
lets the rest of the system clean up the failed backup.
Fixes #99
| snapshot="$(readlink ../in-progress)" | ||
| echo "Error: backup of $GHE_HOSTNAME already in progress in snapshot $snapshot. Aborting." 1>&2 | ||
| if [ -h ../in-progress ]; then | ||
| echo "Error: in progress backup from previous version detected." 1>&2 |
There was a problem hiding this comment.
I think we can improve the error message. Was there a reason to drop $GHE_HOSTNAME from it?
How about:
Error: detected a backup of $GHE_HOSTNAME already in progress.
There was a problem hiding this comment.
Comment seems outdated based on the additional feedback, so going with the last mentioned suggestion.
|
I've left some comments on the error copy, but the functional changes make sense to me. |
|
@michaeltwofish Updated the wording with your suggestion, thanks! |
|
👍 looks good to me. |
dbussink
added a commit
that referenced
this pull request
Oct 9, 2015
Move in progress detection to separate file with pid
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the logic of detecting if a backup is in progress by writing out the snapshot and PID of the process making the backup. This improves the situation where a previous backup was killed in a non-clean way. Clean failures in the sense of a proper exit (also non 0 exit statuses) or using ctrl-c was already handled by traps.
This just removes the in-progress file if the process is no longer running after the backup. It then cleans up the in-progress file and lets the rest of the system clean up the failed backup.
Fixes #99
cc @github/enterprise-infrastructure