-
Notifications
You must be signed in to change notification settings - Fork 268
feat: Add threshold "Less storage space" to warn the user #2157
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
Conversation
buhtz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello fest6,
On behalf of the team, thank you for your contribution and taking the time to improve Back In Time. We appreciate it.
Looks good on a first sight. Would you mind to place the setting in the "Options" tab instead of "Remove & Retention"?
Best,
Christian
|
Thank you for the feedback and kind words! I've moved the settings as requested. |
buhtz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nice and smooth. I like it. Just some minor suggestions.
Please also add a tool tip to the setting and its two widgets. Quick n dirty (with LLM help): Shows a warning when free space on the backup destination disk is less than the specified value.
If the Remove & Retention policy is enabled and old backups are removed based on available free space, this value cannot be lower than the value set in the policy.
I do have a helper function to handle long tooltips, wrap them and organize them in paragraphs:
# Two paragraphs
# Having two translatable strings is on purpose because it avoid escape
# characters (e.g. \n) for non-technical translators.
tooltip = [
_('Shows a warning when free space on the backup destination disk is less '
'than the specified value.'),
_('If the Remove & Retention policy is enabled and old backups are removed '
'based on available free space, this value cannot be lower than the value '
'set in the policy.')
]
qttools.set_wrapped_tooltip(self.suWarnFreeSpace, tooltip)
qttools.set_wrapped_tooltip(self.cbWarnFreeSpace, tooltip)You can try to add a changelog entry if you want. I'll rephrase it later if needed.
Do you plan any further steps within or after this PR?
We can merge it in this state and going further in a new PR.
The thing is that the warning currently occurres only when check-config command is used on the CLI.
I would like to have warning in the logs and notify system, when BIT runs via cron in background or without GUI in CLI. And additionally a warning dialog when starting (but before running) a backup job manually in the GUI. The GUI then should warn and ask the user to proceed with the backup or not.
But this can be realized in the next PR.
|
Thank you for your feedback! I have made the requested changes, except for the one regarding the If everything looks good, please feel free to merge this PR, as I do not have any further plans for this one. |
|
Hello fest6,
The warning dialog itself is still missing.
That is OK. Then I'll take over from here and finalize the PR. Thank you very much. |
|
According to your plan described in #2110 the first of three items are implemented with this PR. I put this PR on hold because the other need to get implemented also. |
|
This is it. I implemented the rest. |
Blocked by implementing details described in #2110.
Added a new config giving a threshold to warn the user about less free space.
Implemented GUI settings panel for adjustments.
Close #2110