Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@fest6
Copy link
Contributor

@fest6 fest6 commented May 19, 2025

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

@buhtz buhtz added the PR: Modifications requested Maintenance team requested modifications and waiting for their implementation label May 19, 2025
@buhtz buhtz added this to the 1.6.0 (upcoming next) milestone May 19, 2025
Copy link
Member

@buhtz buhtz left a 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

@fest6
Copy link
Contributor Author

fest6 commented May 20, 2025

Thank you for the feedback and kind words! I've moved the settings as requested.

Copy link
Member

@buhtz buhtz left a 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.

common/config.py Outdated Show resolved Hide resolved
qt/manageprofiles/tab_options.py Outdated Show resolved Hide resolved
@fest6
Copy link
Contributor Author

fest6 commented Jun 5, 2025

Thank you for your feedback! I have made the requested changes, except for the one regarding the enabled field.

If everything looks good, please feel free to merge this PR, as I do not have any further plans for this one.

@buhtz
Copy link
Member

buhtz commented Jun 5, 2025

Hello fest6,
thank you for the modifications.

If everything looks good, please feel free to merge this PR

The warning dialog itself is still missing.

as I do not have any further plans for this one.

That is OK. Then I'll take over from here and finalize the PR.

Thank you very much.
Regards,
Christian

@buhtz
Copy link
Member

buhtz commented Jun 5, 2025

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.

@buhtz buhtz removed the PR: Modifications requested Maintenance team requested modifications and waiting for their implementation label Jun 5, 2025
@buhtz buhtz changed the title feat: Add threshold "Less storage space" to warn the user [pending] feat: Add threshold "Less storage space" to warn the user Jun 5, 2025
buhtz added a commit that referenced this pull request Jun 9, 2025
@buhtz buhtz changed the title [pending] feat: Add threshold "Less storage space" to warn the user feat: Add threshold "Less storage space" to warn the user Jun 9, 2025
@buhtz
Copy link
Member

buhtz commented Jun 9, 2025

This is it. I implemented the rest.

@buhtz buhtz added the PR: Merge after creative-break Merge after creative-break (min. 1 week) label Jun 9, 2025
@buhtz buhtz merged commit 307ca5f into bit-team:dev Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: Merge after creative-break Merge after creative-break (min. 1 week)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

threshold "Less storage space" to warn the user

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.