Conversation
m3nu
force-pushed
the
exclusions-tab-enhancements
branch
from
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
| 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.
.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.
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.
See related comment for model
src/vorta/assets/UI/excludedialog.ui
Outdated
| </item> | ||
| </layout> | ||
| </widget> | ||
|
|
There was a problem hiding this comment.
Why are we adding a new line here?
shivansh02
force-pushed
the
exclusions-tab-enhancements
branch
from
56d5e81 to
26bb119
Compare
shivansh02
force-pushed
the
exclusions-tab-enhancements
branch
from
cb3bec4 to
898dea1
Compare
src/vorta/store/migrations.py
Outdated
| ), | ||
| ) | ||
|
|
||
| if current_schema.version < 23: |
There was a problem hiding this comment.
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.
Doesn't this mean that all existing exclude settings the user had are now disabled?
There was a problem hiding this comment.
@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.
Yes, I would check them by default.
|
I think we're good now. Let me run it again locally tonight. |
2 participants
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_populatedto 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.