Fix for #1491 (error user callback not called sometimes) and #489 (killing rsync not handled gracefully / rsync exit code not evaluated) #1502
Conversation
Fix bug: Taking a snapshot reports `rsync` errors now even if no snapshot was taken (#1491) Fix bug: takeSnapshot() recognizes errors now by also evaluating the rsync exit code (#489) Fixes related problem: Killing `rsync` was not handled gracefully (by ignoring the rsync exit code) Fix bug: The error user-callback is now always called if an error happened during taking a snapshot (#1491) Improvement: Introduce new error codes for the "error" user callback (as part of #1491): 5: Error while taking a snapshot. 6: New snapshot taken but with errors. Improvement: The `rsync` exit code is now contained in the snapshot log. Example: [E] Error: 'rsync' ended with exit code -9 (negative values are signal numbers, see 'kill -l')
CHANGES
Outdated
| * Fix bug: takeSnapshot() recognizes errors now by also evaluating the rsync exit code (#489) | ||
| Fixes related problem: Killing `rsync` was not handled gracefully (by ignoring the rsync exit code) | ||
| * Fix bug: The error user-callback is now always called if an error happened during taking a snapshot (#1491) | ||
| * Improvement: Introduce new error codes for the "error" user callback (as part of #1491): |
There was a problem hiding this comment.
Based on changelog standards I would name this a "Feature".
There was a problem hiding this comment.
Do you still found the link to the changelog standards (I have lost oversight here).
I would like to add it at the top of the changelog file keep it "in sight" whenever I change the changelog...
There was a problem hiding this comment.
In my own projects where I also decided to write markdown changelog I do use
- https://common-changelog.org
- https://keepachangelog.com
- and slightly related https://www.conventionalcommits.org
- beside of that I summarize the important aspects and examples in my own Zettelkasten (which is not public available)
I didn't want to bother you folks with that details. And myself I'm also quit new to handling things like this. So I'm still in experimenting period and try out what fits best to me.
While getting in touch with that standards I also read about auto-generating changelogs from git commit history. But I don't like that approach because the target readers for commit messages and changelog messages are IMHO different. The first is more technical based and the latter should focus on behavior of the application.
| @@ -91,14 +91,19 @@ def error(self, code, message): | |||
|
|
|||
| Known error codes: | |||
|
|
|||
| 1. No or no valid configuration | |||
| 1: No or no valid configuration | |||
There was a problem hiding this comment.
I assume that new codes should be mentioned in the user-callback repo also.
There was a problem hiding this comment.
Right, I forgot to update the README.md there!
I will update this too today
| @@ -129,6 +129,24 @@ def takeSnapshotMessage(self): | |||
|
|
|||
| #TODO: make own class for takeSnapshotMessage | |||
| def setTakeSnapshotMessage(self, type_id, message, timeout = -1): | |||
| """Update the status message of the active snapshot creation job | |||
There was a problem hiding this comment.
Awesome. Never really get what this function does! 🤣
| if ret_error: | ||
| logger.error('New snapshot taken but errors detected', self) | ||
| self.config.PLUGIN_MANAGER.error(6, sid.displayID) # Fixes #1491 | ||
| ret_error = False # Why ignore errors now? |
There was a problem hiding this comment.
Not sure about this but your arguments sounds fine to me.
|
Thanks a lot for that work and all the comments and improved docu. This will help a lot in the future. |
aryoda
changed the title
rsyncerrors now even if no snapshot was taken(user-callback is not called properly when an error occurs #1491)
(Killing rsync not handled gracefully / rsync exit code not evaluated #489)
Fixes related problem: Killing
rsyncwas not handled gracefully (by ignoring the rsync exit code)(user-callback is not called properly when an error occurs #1491)
(as part of user-callback is not called properly when an error occurs #1491):
5: Error while taking a snapshot.
6: New snapshot taken but with errors.
rsyncexit code is now contained in the snapshot log. Example:[E] Error: 'rsync' ended with exit code -9 (negative values are signal numbers, see 'kill -l')
Notes
I did only apply required changes to fix the issues since the
takeSnapshot()function is the "heart" of BiTand has a quite complicated logic (many
ifs, legacy parameter passing with tuples instead of named classes etc.).Still I have added a lot of TODOs, comments and docstrings to make the logic better understandable (I hope).
Testing
I have done many manual tests & debugging with different signals,
rsyncerrors and with and without theTake a new snapshot whether there were changes or notoption.The unit tests do succeed (
make test) on my local machine.