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

Bugfix: Exit bitpocket when a rsync command fails #78

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

raphyduck
Copy link

Also includes PR #77
Also uses the rsync option "--partial-dir", which is safe to use in case of network error but saves some bandwidth for error-prone network connections

@greezybacon
Copy link
Collaborator

Ok, so I really like your idea. There's a possible security risk with the --partial-dir option if other users have write access to it, so maybe we should have a check for that? Or forceably change it to R/W only by the user executing the bitpocket script?:

IMPORTANT: the --partial-dir should not be writable by other users or it is a security risk.
E.g. AVOID "/tmp".

Secondly, it requires extra space on the destination machine. Perhaps it should only be used for only the pull or push or both? If transferring large files, I could see where this might create problems?

Thirdly, I think we may want to combine the idea with the --delay-updates feature, which will hold all updates in the partial-dir location until the end of the sync. Then it will move all the new files into place. This is the way most systems do software updates. It also implements a transaction mechanism, so if the sync is interrupted, then none of the files are updated remotely. Again, this would likely not fit everyone's use case, but when synchronizing folders from where scripts and software is executed, this would be a very good option.

@raphyduck raphyduck closed this Oct 20, 2018
@raphyduck raphyduck reopened this Oct 20, 2018
@raphyduck
Copy link
Author

Ok @greezybacon Is this what you had in mind? I implemented it only on the pull side, but the main point of the PR is to really exit in case of rsync error, which on my machines was not happening.

@greezybacon
Copy link
Collaborator

OK @raphyduck I see your point with exiting based on rsync failures. I've opened #79 to handle that issue independently. Would you mind testing and giving an opinion?

I don't like pork merges (where a PR includes stuff outside the scope of the PR -- eg. including #77), and I don't like merging a dozen unnecessary "mc" commits. So, I'll try rebasing and opening a separate PR for the transaction idea (with the --partial idea).

@raphyduck
Copy link
Author

Great thanks @greezybacon :)

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