Import/export settings (or bootstrap with default config) #955

phihos · 2021-04-21T22:20:35Z

Continuing the work of #954.

Left to do:

phihos · 2021-04-29T00:22:17Z

It took me longer than expected (last week was busy) but I made some progress. There is now a rudimentary config backup/restore functionality. I plan to support backup/restore of multiple profiles at once.

@m3nu It would be very welcome if you could take a look and tell me if I am going into the right direction.

m3nu · 2021-05-01T02:41:05Z

Good stuff! Basic flow works. Great progress. Some thoughts (may overlap with your todo):

File name/extension to use. Currently .vortabackup. Maybe just .json is clearer.
Setting $PROFILE.json as default filename to save would be nice. Maybe it's possible to fix the extension and users can only enter the name.
I'd strongly suggest to call the feature Import/Export Profile in classes, filenames and tests. Since Vorta is already a Backup app, calling it BackupWindow or RestoreWindow or ConfigBackup will be too ambiguous. Similar to Borg's borg key export/import. Export the key, export the profile. 🤓
The main window is built around Profiles. Change the profile, all the content changes. The buttons next to the profile dropdown also do stuff with a single profile. So I would keep things simple and export the current profile over the export button. This will greatly simplify things because a) no profile selector and list is needed and b) one json file = one profile. If you want to export global settings, this could be an optional checkbox.
You wanted to bootstrap from a default location. This could be triggered by looking for a specific file, if there are no repos (in srv/vorta/models.py) or with a command line argument. Looking in a default location only if no profile exists might be the least intrusive (doesn't add new CLI flags, a GUI shouldn't have CLI flags really). Default location could be ~/.vorta-init.json?
Optionally include password. Good. We may need a dialog to ask for the password if it's not included.
I question whether events and archives should be included. Not useful and maybe outdated. Better if those are fetched when verifying the repo.

…e import

phihos · 2021-05-16T18:50:28Z

Hi @m3nu,

beside the bootstrapping I think I ticked off all items on your list. Could you take another look?
Also I have a question about the event log: I understand, that the archives can be recovered from the repo, but that is not true for the event log right? So why should they be excluded from the export?

m3nu · 2021-05-17T04:14:19Z

Perfect! Please allow me 1-2 days for testing and review.

Then we can merge in time for the next 0.7.6 release.

phihos · 2021-05-17T06:38:10Z

Great, I still need to fix/implement tests and the bootstrap feature though. I try to get this done within the next two days also.

phihos · 2021-05-22T21:14:12Z

Hi @m3nu, I am almost done. I only need to clarify two things:

Can you answer this question I had earlier?

Also I have a question about the event log: I understand, that the archives can be recovered from the repo, but that is not true for the event log right? So why should they be excluded from the export?

I thought about my bootstrap use-case and at least in my case I need to be able to overwrite existing profiles. I already implemented the possibility to overwrite a preexsiting profile with the same name when importing. I like the idea of using a fixed location with a json file for bootstrapping. Maybe we could extend that to overwriting a profile with the same name when this json file exists. After importing the file needs to be deleted by vorta of course to prevent resetting the profile every time. For me personally doing that silently would be fine, but I could also add a prompt for the user to accept before overwriting the profile from the export file. What do you think?

Edit: I will also mark this PR as ready for review because I think it is a functional unit. We could deal with documentation and bootstrapping in another PR and long running PRs usually do not end well.

m3nu · 2021-05-23T07:52:16Z

I'd say event logs aren't too important to keep around. Personally I never looked at them. We don't copy the log files either.

Overwriting the profile and deleting the import file after sounds good. Since you want to do it non-interactive, you probably do not want a confirmation? It's also very deliberate to place such a bootstrap file. It won't happen by accident.

phihos · 2021-05-23T12:51:49Z

Ok I resolved the outstanding issues. Profiles located at ~/.vorta-init.json will be imported on application startup. I resued some code from the interactive import for that. I also removed the event log from the backup.
@m3nu I think you can now review the final code if when you have the time. What about documentation? Do you wnat to take care of that yourself or would you like me to write it? And if so where?

m3nu · 2021-05-23T15:45:57Z

Perfect. Will look at it first thing tomorrow.

I can do the docs. It’s a separate repo. Will also help me understand it from a user view.

phihos · 2021-05-23T21:39:45Z

I messed up the tests with my last commit. They should pass now.

m3nu · 2021-05-24T07:16:22Z

src/vorta/borg/create.py

@@ -51,13 +51,13 @@ def finished_event(self, result):

    @classmethod
    def pre_post_backup_cmd(cls, params, cmd='pre_backup_cmd', returncode=0):
-        cmd = getattr(params['profile'], cmd)
+        cmd = getattr(params['profile_export'], cmd)


Search & Replace gone overboard here? Also affects a few other files.

Oh how embarassing. Fixing it ASAP.

No worries. Can happen in the heat of battle.

m3nu · 2021-05-24T07:35:51Z

src/vorta/views/main_window.py

@@ -163,6 +182,60 @@ def profile_add_action(self):
        window.profile_changed.connect(self.profile_add_edit_result)
        window.rejected.connect(lambda: self.profileSelector.setCurrentIndex(self.profileSelector.currentIndex()))

+    def profile_export_action(self):
+        window = ExportWindow(profile=self.current_profile)


Suggest to use self.profile() here as reference to the profile saved in the DB. Else updated settings won't be saved during export.

For that to work in need to add BackupProfileMixin to the main window. Shall I do it?

I added it now and it works. But I can change it if you had something different in mind.

src/vorta/borg/init.py

tests/test_importexport.py

m3nu · 2021-05-24T07:46:20Z

Good stuff. 👍 Just noticed a few minor things. This will be ready to merge soon.

I was a bit skeptical on the Import/Export buttons, but after using it, it makes sense.

phihos · 2021-06-03T11:19:43Z

The change was more extensive than anticipated. I did not want to delete the "Default" profile when bootstrapping but prevent its creation instead. Else I fear there could be some unexpected side effects.
What I did:

I moved the creation of the defaut profile from init_db to the main window.
I run init_db at the end of ProfileExport.to_db to fix another issue I encountered: When the export does not contain all the settings the missing setting will not be recreated until after application restart.
The main window has now an optional parameter for where to look for the export file. If it does not exist bootstrapping is silently skipped as before.
I changed the init_db pytest fixture to check for a pytest mark "profile_bootstrap_file" telling it to skip creating default data and initializing the main window with an export file path instead.
I split the existing bootstrap test into two tests: The first only checks if the bootstrap file is deleted properly after import. The second one tells the init_db fixture to use a valid export file for bootstrapping and checks if the app imported it correctly. There is also an assert checking if the bootstrapped profile is the only existing profile.

Beside the tests I also checked the behavior manually. When starting Vorta for the first time and bootstrapping from .vorta-init.json we only have the imported profile. When starting Vorta not the first time and we bootstrap from file preexisting profiles are preserved (if the imported profile name != existing profile name of course).

I can now edit the PR description so the commit message actually describes the feature. Or do you want to do this?

m3nu · 2021-06-03T11:37:08Z

Another option is to only do bootstrapping, if no profile exists yet? Since overwriting things may be surprising for users, if you place the file via MDM.

Reasons for moving stuff around are clear. I'd prefer to have less stuff in MainWindow, but that may make it more complicated. Will see.

phihos · 2021-06-03T12:06:28Z

Another option is to only do bootstrapping, if no profile exists yet?

At least in my use-case it causes more confusion: I work with a self-service app, and I plan to add an item there like "Setup Backup" or something. It should install Vorta and place the bootstrap file. The item then changes to "Update Backup" and when clicked it re-places the bootstrap file. This is for botched configurations or when the backup server's port changes or similar.
If we only do bootstrapping when no profile exists then I would either have to delete the whole database beforehand or tell users to delete the profile before clicking "Update Backup". Both is not ideal I think.
How would you solve this particular case?

m3nu · 2021-06-04T07:37:33Z

How would you solve this particular case?

I don't have a better solution either. Overwriting it is OK, since placing this file is very intentional.

Will continue to understand the changes and merge it now.

m3nu · 2021-06-04T09:56:30Z

Re-arranged a few things to isolate this feature more. Just so it's all in one place, if someone needs to make changes.

For the test, I just removed the pytest-mark thing in conftest.py and mocked Path.unlink() instead to achieve the same thing (test if profile is imported and file deleted).

Then for the hardcoded ~/.vorta-init.json path, I moved it to vorta.config, since we already have some other path configs there. Just to have it all in one place.

Hope you agree with my changes. It's really just how I like to structure things for the future. There was nothing wrong before. Maybe I even broke something, so please check that it all works as expected still.

After that, let's merge this and do a release to get some feedback (Users don't usually test the master branch, so I do a release and then a bugfix release shortly after, if needed 😄)

phihos · 2021-06-05T10:51:28Z

The changes are fine to me. And it totally makes sense that you keep an eye on the overall architecture. I wouldn't do it differently with my own projects :-)

I checked out your changes and tested the bootstrapping. Still seems to work. On bootstrap the user is not notified about the import any more. Is that on purpose?

m3nu · 2021-06-05T10:57:23Z

On bootstrap the user is not notified about the import any more. Is that on purpose?

Not really. Maybe I cleaned up too much. Let me put it back.

…) since this is a class method

m3nu · 2021-06-05T11:09:43Z

I put back the notification and added a log as well.

m3nu · 2021-06-05T11:12:44Z

Seems we're done here. Next up are some docs on how to use the bootstrapped import. Not very complicated though.

phihos · 2021-06-05T11:22:33Z

That is great news! Can't wait for your next release 🎉

m3nu · 2021-06-05T11:37:03Z

Docs: borgbase/vorta.borgbase.com#24

Please let me know if you want to add anything. This will be below Usage

m3nu · 2021-06-05T11:39:20Z

Release tomorrow morning or so. We have enough features by now.

m3nu · 2021-06-08T15:37:24Z

Release done, @phihos : https://github.com/borgbase/vorta/releases/tag/v0.7.6

Stokel · 2022-06-30T08:54:52Z

Just discovered vorta and I noticed, that when exporting my profile, that only the currently selected repository is exported into the json file. Is this working as intended? Is there a way to export the complete profile/configuration to include all repositories?

real-yfprojects · 2022-06-30T19:07:49Z

Just discovered vorta and I noticed, that when exporting my profile, that only the currently selected repository is exported into the json file. Is this working as intended? Is there a way to export the complete profile/configuration to include all repositories?

Currently only the current profile is exported along with the general settings. If you feel like vorta should show a different behaviour, which is very legitimate, please open a new issue.

phihos marked this pull request as draft April 21, 2021 22:20

phihos mentioned this pull request Apr 21, 2021

Import/export settings (or bootstrap with default config) #954

Closed

First working config backup & restore

fbc8219

phihos added 4 commits May 16, 2021 12:37

Rename profile backup to profile export and profile restore to profil…

2ea63d8

…e import

Set <profile name>.json as default profile export file name

f1e673b

Improve UX for profile import

1ceae42

Fix repo being deleted for existing profiles after profile import

50b7f7b

phihos added 5 commits May 19, 2021 00:05

Fix tests

a2e425e

Merge branch 'master' into issue/import-export

332f87f

Add possibility to overwrite an existing profile when importing

02e8769

Separate import window into own module

05ca5c4

Improve text in export window

b70c16e

phihos marked this pull request as ready for review May 22, 2021 21:18

Fix lints

5ea0c17

phihos added 2 commits May 23, 2021 14:40

Add bootstrap import from ~/.vorta-init.json

da13a19

Remove event log fromprofile import/export

c456058

Fix lint

40f858c

Fix test data for test_importexport

7f62ef1

m3nu requested changes May 24, 2021

View reviewed changes

phihos requested a review from m3nu June 3, 2021 11:20

Merge branch 'master' into issue/import-export

c54458b

m3nu added 7 commits June 4, 2021 13:20

Nit-pick, more isolation

0dcb8b8

Mock unlink() instead of copying file

ab9b84c

Remove leftover fixture

cf26dd0

Remove leftover imports

36bd0f7

Remove leftover args

48adb02

Lint comment

eba80d7

Avoid comment change

1047d91

m3nu previously approved these changes Jun 4, 2021

View reviewed changes

Change signature from_json(self, filename) to from_json(cls, filename…

27ca7cc

…) since this is a class method

phihos dismissed m3nu’s stale review via 27ca7cc June 5, 2021 10:59

Add back notification and logging after bootstrapped import.

73dc2ac

m3nu approved these changes Jun 5, 2021

View reviewed changes

m3nu merged commit 5192bd1 into borgbase:master Jun 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import/export settings (or bootstrap with default config) #955

Import/export settings (or bootstrap with default config) #955

phihos commented Apr 21, 2021 •

edited

Loading

phihos commented Apr 29, 2021

m3nu commented May 1, 2021

phihos commented May 16, 2021

m3nu commented May 17, 2021

phihos commented May 17, 2021

phihos commented May 22, 2021 •

edited

Loading

m3nu commented May 23, 2021

phihos commented May 23, 2021 •

edited

Loading

m3nu commented May 23, 2021

phihos commented May 23, 2021

m3nu May 24, 2021

phihos May 24, 2021

m3nu May 24, 2021

m3nu May 24, 2021

phihos May 24, 2021

phihos May 24, 2021

m3nu commented May 24, 2021

phihos commented Jun 3, 2021 •

edited

Loading

m3nu commented Jun 3, 2021

phihos commented Jun 3, 2021 •

edited

Loading

m3nu commented Jun 4, 2021

m3nu commented Jun 4, 2021 •

edited

Loading

phihos commented Jun 5, 2021

m3nu commented Jun 5, 2021

m3nu commented Jun 5, 2021 •

edited

Loading

m3nu commented Jun 5, 2021

phihos commented Jun 5, 2021

m3nu commented Jun 5, 2021

m3nu commented Jun 5, 2021

m3nu commented Jun 8, 2021

Stokel commented Jun 30, 2022

real-yfprojects commented Jun 30, 2022

Import/export settings (or bootstrap with default config) #955

Import/export settings (or bootstrap with default config) #955

Conversation

phihos commented Apr 21, 2021 • edited Loading

phihos commented Apr 29, 2021

m3nu commented May 1, 2021

phihos commented May 16, 2021

m3nu commented May 17, 2021

phihos commented May 17, 2021

phihos commented May 22, 2021 • edited Loading

m3nu commented May 23, 2021

phihos commented May 23, 2021 • edited Loading

m3nu commented May 23, 2021

phihos commented May 23, 2021

m3nu May 24, 2021

Choose a reason for hiding this comment

phihos May 24, 2021

Choose a reason for hiding this comment

m3nu May 24, 2021

Choose a reason for hiding this comment

m3nu May 24, 2021

Choose a reason for hiding this comment

phihos May 24, 2021

Choose a reason for hiding this comment

phihos May 24, 2021

Choose a reason for hiding this comment

m3nu commented May 24, 2021

phihos commented Jun 3, 2021 • edited Loading

m3nu commented Jun 3, 2021

phihos commented Jun 3, 2021 • edited Loading

m3nu commented Jun 4, 2021

m3nu commented Jun 4, 2021 • edited Loading

phihos commented Jun 5, 2021

m3nu commented Jun 5, 2021

m3nu commented Jun 5, 2021 • edited Loading

m3nu commented Jun 5, 2021

phihos commented Jun 5, 2021

m3nu commented Jun 5, 2021

m3nu commented Jun 5, 2021

m3nu commented Jun 8, 2021

Stokel commented Jun 30, 2022

real-yfprojects commented Jun 30, 2022

phihos commented Apr 21, 2021 •

edited

Loading

phihos commented May 22, 2021 •

edited

Loading

phihos commented May 23, 2021 •

edited

Loading

phihos commented Jun 3, 2021 •

edited

Loading

phihos commented Jun 3, 2021 •

edited

Loading

m3nu commented Jun 4, 2021 •

edited

Loading

m3nu commented Jun 5, 2021 •

edited

Loading