feat: exclude gui #1742

diivi · 2023-06-27T21:08:30Z

Description

Progress updates for #907 will be added here. This is the base implementation which will allow the user to add exclude rules, select exclude presets to include, and edit/copy/paste the exclusions from a "Raw" tab.

TODO:

Presets Tab and a starting json file for presets.
Raw tab.
Actually pass the data to borg.
Exclude if Present Dialog.

Related Issue

#907

Motivation and Context

Makes it simpler for the user to add excludes with a more intuitive interface.

How Has This Been Tested?

Testing this manually for all scenarios that I can think of.

Screencast.from.21-08-23.01.12.55.AM.IST.webm

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have read the CONTRIBUTING guide.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

Follow ups

Preview excludes (dialog)
Edit custom patterns inline

src/vorta/views/exclude_dialog.py

Hofer-Julian · 2023-07-11T07:54:58Z

src/vorta/assets/exclusion_presets/browsers.json

+            "fm:/snap/chromium/*/.local/share/"
+        ],
+        "tags":["application:chromium", "type:browser", "os:linux"],
+        "author": "Divi"


Is the author key needed for anything?

Nope, only to attribute the author.

Does that make sense? Contributors might modify presets in the future. Furthermore since the author isn't shown in the GUI.

@m3nu I had the same thought regarding edits and revisions.

src/vorta/assets/exclusion_presets/browsers.json

src/vorta/assets/exclusion_presets/dev.json

Hofer-Julian

I've now tested the current WIP. I like the direction this is going.
My suggestions:

Let's remove the Raw tab. The combination of Custom to enter and Preview to view covers all use cases of Raw as far as I can tell
Reorder to Presets, Custom, Preview
Stop adding the fm prefix and link to https://docs.python.org/3/library/fnmatch.html. That way we avoid overwhelming the users

diivi · 2023-07-11T09:54:04Z

I am fine with the other points and will wait for other reviews, but I think the Raw tab might be useful for adding multiple patterns together, for example just pasting them in from somewhere, or sharing a list of patterns between users (copy from the preview tab and paste it in the raw tab).

I'd prefer the raw tab to quickly add patterns that I'll always need instead of adding patterns again and again through the custom tab.

But that's just one reason to support the Raw tab, I'm open to feedback.

Hofer-Julian · 2023-07-11T10:00:09Z

I think the Raw tab might be useful for adding multiple patterns together, for example just pasting them in from somewhere, or sharing a list of patterns between users (copy from the preview tab and paste it in the raw tab).

I forgot about that use case. It's probably fine to keep it then

One thing that I've just noticed is that Preview looks a bit too similar to the input tabs.
We probably want to either highlight it somehow or make the view always visible and drop the tab.
Unsure about that one though and I wouldn't consider it a blocker

real-yfprojects · 2023-07-11T13:09:07Z

Stop adding the fm prefix and link to https://docs.python.org/3/library/fnmatch.html. That way we avoid overwhelming the users

That is a good idea GUI wise, however our preset json files should keep the prefixes. We should also support adding non-fm-style patterns.

diivi · 2023-07-11T13:31:23Z

That is a good idea GUI wise.

This is just shown in the Preview tab, which should show what gets passed to borg directly anyway.

We should also support adding non-fm-style patterns.

It's supported, anything that you can put inside a Borg exclusion file is supported (it's visible for the custom presets right now too).

Hofer-Julian · 2023-07-11T13:34:52Z

I still feel that dropping the fm prefix also in the backend is the sweetspot here. It is the default, so the behavior doesn't change, it still allows other styles to be specified and we don't need logic to hide complexity from the user.

diivi · 2023-07-11T13:49:59Z

It is the default, so the behavior doesn't change, it still allows other styles to be specified and we don't need logic to hide complexity from the user.

Oh okay, that sounds good. I thought I'll have to hide it from the user or something. But yes, I can remove it entirely and use prefixes only for the other types (I don't know many use cases so I'll probably just leave it for later).

real-yfprojects · 2023-07-11T16:14:10Z

It is the default, so the behavior doesn't change

Well borg changed the default style in the past afaik.

Hofer-Julian · 2023-07-11T17:04:59Z

Maybe you can help here @ThomasWaldmann:
Is it safe to assume that Fnmatch will continue to be the default style for --exclude and --exclude-from?

ThomasWaldmann · 2023-07-11T17:32:27Z

@Hofer-Julian There is currently no change planned. But I recently opened a ticket about "patterns docs and reality", so there might be some work on that in general for borg2.

Hofer-Julian · 2023-07-11T17:39:13Z

@Hofer-Julian There is currently no change planned. But I recently opened a ticket about "patterns docs and reality", so there might be some work on that in general for borg2.

Thank you, @ThomasWaldmann! (that's the link for the people who are curious too borgbackup/borg#7634)

That sounds like @real-yfprojects suggestion to specify the fm prefix is indeed the best option.

I thought I'll have to hide it from the user or something

@diivi I think you don't have to go out of your way to hide the prefix from the user

src/vorta/assets/exclusion_presets/browsers.json

src/vorta/store/models.py

src/vorta/views/exclude_dialog.py

src/vorta/store/models.py

real-yfprojects · 2023-07-14T07:48:43Z

How do we handle changes to the presets in the future? The user might wonder that something is suddenly excluded or included. Or he won't notice at all.

Hofer-Julian · 2023-07-14T09:01:29Z

How do we handle changes to the presets in the future? The user might wonder that something is suddenly excluded or included. Or he won't notice at all.

I would expect that all patterns are disabled per default.
So if we add new patterns, nothing will change for the user.

diivi · 2023-07-14T09:07:02Z

I would expect that all patterns are disabled per default. So if we add new patterns, nothing will change for the user.

I think they mean the case where I enable a preset and then later someone changes that preset from Vorta source to add/remove a pattern. I won't know which files are excluded.

I think they can just check the preview tab for the final exclusions, as they should.

real-yfprojects · 2023-07-14T09:45:24Z

I think they can just check the preview tab for the final exclusions, as they should.

Requiring users to recheck their excludes after every update isn't very user friendly, is it?

diivi · 2023-07-14T11:54:49Z

True, and for scheduled backups people might not even check the preview tab. I don't really have a solution in mind though :/

real-yfprojects · 2023-07-18T18:54:35Z

True, and for scheduled backups people might not even check the preview tab. I don't really have a solution in mind though :/

Possible solutions include:

including such changes only in breaking releases and having a section with breaking changes in the release message
transferring the rules of the changed preset to the user defined rules when updating.
Showing a prominent warning on startup after update
Only adding new presets (labelled v2) instead of changing existing ones

Combinations of these could make sense as well.

diivi · 2023-08-24T20:49:00Z

Screencast.from.25-08-23.02.17.33.AM.IST.webm

Works on my end with correct distinction.

diivi · 2023-08-24T21:05:09Z

@jetchirag can you help me figure out where and how I can move the 2 tests for this feaure? I put them in the tests folder to start, but after your PR got merged they should probably be changed a little and go inside one of /unit or /integration directories.

m3nu · 2023-08-25T12:06:47Z

Just noticed that all my exclude rules went missing after testing this PR. I have some backups of my settings, so no problem. Let's just make sure this doesn't happen for real users.

jetchirag · 2023-08-25T12:51:02Z

@diivi Since this is only testing the interface, you can simply move it to tests/unit. I don't think you need to change anything in the test itself.

diivi · 2023-08-26T10:45:28Z

@jetchirag Just moving these files to the unit folder does not work:

=============================================================== FAILURES ===============================================================
___________________________________________________ test_exclusion_preview_populated ___________________________________________________
CALL ERROR: Exceptions caught in Qt event loop:
________________________________________________________________________________
Traceback (most recent call last):
  File "/home/divyansh/Desktop/vorta/src/vorta/views/schedule_tab.py", line 106, in <lambda>
    self.app.scheduler.schedule_changed.connect(lambda pid: self.draw_next_scheduled_backup())
  File "/home/divyansh/Desktop/vorta/src/vorta/views/schedule_tab.py", line 175, in draw_next_scheduled_backup
    status = self.app.scheduler.next_job_for_profile(self.profile().id)
  File "/home/divyansh/Desktop/vorta/src/vorta/store/models.py", line 238, in profile
    return BackupProfileModel.get(id=self.window().current_profile.id)
RuntimeError: wrapped C/C++ object of type ScheduleTab has been deleted
__________________________________________________________________________

jetchirag · 2023-08-27T07:52:19Z

@diivi Check commit a56af3a for fix

real-yfprojects

Looking all good, but the implementation can be enhanced. Currently exclude_dialog.py and exclude_if_present_dialog.py share a lot of code -> redundancy. For example you don't have to maintain to implementations of MandatoryInputItemModel. Instead you can reference the same class in both files. Regarding the duplicate code in ExcludeDialog and ExcludIfPresentDialog you can inheritance to fix that. Move the duplicate code into a common super class and add the custom behaviour like different ui strings by overriding methods in the subclasses.

src/vorta/store/migrations.py

diivi · 2023-09-01T21:24:44Z

Move the duplicate code into a common super class

I tried doing this first, but got stuck with multiple inheritance - https://stackoverflow.com/questions/3277367/how-does-pythons-super-work-with-multiple-inheritance. In my case the exclusion dialogs had three classes to inherit from - (ExcludeDialogBase, ExcludeDialogUi, ExclusionDialogBase - which is the superclass). But I could not ininitialise all three with the same params. And qt forces me to use super.

Also, I tried moving the setupUi and ExcludeDialogBase, ExcludeDialogUi classes to the superclass, but I could not understand the order in which they are supposed to be called.

Do you have an example of Qt widget superclasses in the Vorta codebase?

real-yfprojects · 2023-09-03T08:27:16Z

Do you have an example of Qt widget superclasses in the Vorta codebase?

You can have a look how @jetchirag has done it in #1749 for DiffResultDialog and ExtractDialog.

m3nu · 2023-09-19T17:31:26Z

Is this still being worked on or abandoned? If the latter, we would need to see if we can finish the work somehow.

diivi · 2023-09-19T18:14:49Z

I'll resume this, got a little busy with course work. I have exams during the first week of Ocotber, so I'll try to get this merged after that. I'll see if I can take out an hour everyday to complete this before that.

m3nu · 2023-09-19T19:08:39Z

Great! Any help, let us know here. Would love to see this included in the next release.

diivi · 2023-10-13T14:53:58Z

I'm getting some errors with changing the presets list in the exclusion dialog, probably made an error in extending the Base Class. Will keep investigating.

m3nu · 2023-10-25T06:36:25Z

Hey Divi,

Can we merge a simplified version of this with the original scope? I know we changed the scope last minute by including managing those "Exclude if present" files and then asking for subclassing those (both are correct in the long-term). If this is proving too hard (it's also a very niche feature), I'd just skip it and do a new branch and PR with just file exclusions, as originally planned. Same as what you present in your results: https://github.com/diivi/GSoC23-PythonSoftwareFoundation#project-achievements

Let me know what you think. We can then add "Exclude if present" later or the few users who rely on it can use CLI args.

diivi · 2023-10-30T03:45:46Z

Yup, sorry I've been a little busy with college and my internship hunt. I'll try to revert to the original approved version when I get a chance.

m3nu · 2023-11-11T08:13:25Z

@diivi , can you revert this to the exclude GUI only state? With the "exclude if present" text area field removed for now (but data settings still there). I think this would be a good starting conclusion of the project you also worked hard on and probably want to see merged and maintained.

If you're busy and already left for new endeavors, I'll make the change some time after Wednesday next week.

Thanks!

m3nu · 2023-11-24T21:20:50Z

Closing this, since a less ambitious version was merged via #1846. Thanks again to @diivi for contributing this!

diivi added 4 commits June 28, 2023 02:10

feat: exclude gui

21c86cd

add and remove patterns from custom table

e46e1f1

database integration for adding and removing custom patterns

a2cd061

complete custom presets tab frontend

e4b2640

diivi changed the title ~~Feat/exclude gui~~ feat: exclude gui Jun 27, 2023

diivi added 2 commits June 30, 2023 00:52

remove unnecessary array dependency

28c082d

custom presets

bd2367d

diivi commented Jul 6, 2023

View reviewed changes

src/vorta/views/exclude_dialog.py Outdated Show resolved Hide resolved

Hofer-Julian reviewed Jul 11, 2023

View reviewed changes

real-yfprojects reviewed Jul 14, 2023

View reviewed changes

diivi mentioned this pull request Jul 18, 2023

FR: Improve Test Coverage: A Test Plan #1754

Closed

diivi added 2 commits July 18, 2023 22:47

remove all leading slashes and work on code review

1b4548f

Merge branch 'master' into feat/exclude-gui

a1fc018

diivi added 2 commits August 25, 2023 02:20

update exclude if present window title

423aa4d

add test for exclude if present dialog

cae7ba4

diivi added 2 commits August 28, 2023 02:42

move tests

1cf2223

Merge branch 'master' into feat/exclude-gui

5d71208

diivi requested a review from real-yfprojects August 27, 2023 21:13

real-yfprojects reviewed Sep 1, 2023

View reviewed changes

src/vorta/store/migrations.py Outdated Show resolved Hide resolved

src/vorta/store/migrations.py Outdated Show resolved Hide resolved

src/vorta/store/migrations.py Outdated Show resolved Hide resolved

remove duplicate MandatoryInputItemModel

15c87e6

diivi added 2 commits October 13, 2023 19:40

Merge branch 'master' into feat/exclude-gui

25a44d6

partial for file exclusions

c74853d

Merge branch 'master' into feat/exclude-gui

85cff58

m3nu marked this pull request as draft November 16, 2023 13:45

Merge branch 'master' into feat/exclude-gui

3566e0d

m3nu mentioned this pull request Nov 16, 2023

Exclude GUI II (without exclude-if-present) #1846

Merged

m3nu closed this Nov 24, 2023

feat: exclude gui #1742

feat: exclude gui #1742

Conversation

diivi commented Jun 27, 2023 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Follow ups

Hofer-Julian Jul 11, 2023

Choose a reason for hiding this comment

diivi Jul 11, 2023

Choose a reason for hiding this comment

real-yfprojects Jul 11, 2023

Choose a reason for hiding this comment

diivi Jul 11, 2023

Choose a reason for hiding this comment

Hofer-Julian left a comment • edited Loading

Choose a reason for hiding this comment

diivi commented Jul 11, 2023 • edited Loading

Hofer-Julian commented Jul 11, 2023

real-yfprojects commented Jul 11, 2023

diivi commented Jul 11, 2023

Hofer-Julian commented Jul 11, 2023

diivi commented Jul 11, 2023

real-yfprojects commented Jul 11, 2023

Hofer-Julian commented Jul 11, 2023

ThomasWaldmann commented Jul 11, 2023

Hofer-Julian commented Jul 11, 2023

real-yfprojects commented Jul 14, 2023

Hofer-Julian commented Jul 14, 2023

diivi commented Jul 14, 2023 • edited Loading

real-yfprojects commented Jul 14, 2023

diivi commented Jul 14, 2023

real-yfprojects commented Jul 18, 2023

diivi commented Aug 24, 2023

diivi commented Aug 24, 2023

m3nu commented Aug 25, 2023

jetchirag commented Aug 25, 2023

diivi commented Aug 26, 2023 • edited Loading

jetchirag commented Aug 27, 2023 • edited Loading

real-yfprojects left a comment

Choose a reason for hiding this comment

diivi commented Sep 1, 2023

real-yfprojects commented Sep 3, 2023

m3nu commented Sep 19, 2023

diivi commented Sep 19, 2023

m3nu commented Sep 19, 2023

diivi commented Oct 13, 2023

m3nu commented Oct 25, 2023

diivi commented Oct 30, 2023

m3nu commented Nov 11, 2023

m3nu commented Nov 24, 2023

diivi commented Jun 27, 2023 •

edited

Loading

Hofer-Julian left a comment •

edited

Loading

diivi commented Jul 11, 2023 •

edited

Loading

diivi commented Jul 14, 2023 •

edited

Loading

diivi commented Aug 26, 2023 •

edited

Loading

jetchirag commented Aug 27, 2023 •

edited

Loading