-
Notifications
You must be signed in to change notification settings - Fork 137
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
Exclude If Present #2016
Exclude If Present #2016
Conversation
a28d690
to
56d5e81
Compare
Testing this locally now. In the future, you can also rebase a new PR if there were commits in the meantime. |
Works as expected locally and passes the correct args to Borg. I would still suggest to add some common example files in the dialog, but keep them unselected. Like the ones shown in Borg's docs. |
I added two files (.nobackup and .vortaignore). Did these not show up for you? |
src/vorta/store/models.py
Outdated
@@ -79,7 +79,7 @@ class BackupProfileModel(BaseModel): | |||
ssh_key = pw.CharField(default=None, null=True) | |||
compression = pw.CharField(default='lz4') | |||
exclude_patterns = pw.TextField(null=True) | |||
exclude_if_present = pw.TextField(null=True) | |||
exclude_if_present = pw.TextField(null=True, default="[] .nobackup\n[] .vortaignore") |
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.
.nobackup
is fine, but I wouldn't introduce a new standard, like .vortaignore
. Why not add CACHEDIR.TAG
, which is an existing standard to support the related Borg flag.
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.
Wasn't aware about CACHEDIR.TAG, added it now.
src/vorta/store/migrations.py
Outdated
migrator.add_column_default( | ||
BackupProfileModel._meta.table_name, | ||
'exclude_if_present', | ||
pw.TextField(default="[] .nobackup\n[] .vortaignore"), |
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.
See related comment for model
src/vorta/assets/UI/excludedialog.ui
Outdated
@@ -137,6 +177,7 @@ | |||
</item> | |||
</layout> | |||
</widget> | |||
|
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.
Why are we adding a new line here?
56d5e81
to
26bb119
Compare
…ansh02/vorta into exclusions-tab-enhancements
cb3bec4
to
898dea1
Compare
src/vorta/store/migrations.py
Outdated
@@ -250,6 +250,17 @@ def run_migrations(current_schema, db_connection): | |||
), | |||
) | |||
|
|||
if current_schema.version < 23: |
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.
I believe we don't need a migration here, since the default comes from models.py
. This also avoids duplicating the same content.
src/vorta/views/exclude_dialog.py
Outdated
else: | ||
item.setText(pattern) | ||
item.setCheckable(True) | ||
item.setCheckState(Qt.CheckState.Unchecked) |
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.
Doesn't this mean that all existing exclude settings the user had are now disabled?
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.
@m3nu That's true, I didn't take the existing ones into account. Should I try to make them checked by default?
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.
Yes, I would check them by default.
I think we're good now. Let me run it again locally tonight. |
Description
Exclude directories where specified files are present.
Related Issue
#1863
Motivation and Context
How Has This Been Tested?
Tested manually by running backups. Selected filenames get appended to borg create command as "--exclude-if-present filename". Modified
test_exclusion_preview_populated
to test preview tab.Screenshots (if appropriate):
Types of changes
Checklist:
I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.