[Enhancement] Avoid blocking UI while adding existing repo #1643

DaffyTheDuck · 2023-03-12T12:06:53Z

Description

Related Issue

Closes #1306

Motivation and Context

When adding a existing repository the UI freezes, this change might solve this issue

How Has This Been Tested?

I've tested this by adding a repository which is used to crash the UI

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.

DaffyTheDuck · 2023-03-12T12:08:23Z

Hi! @real-yfprojects , is this right ? like the UI is not freezing now, but after reverting back to the old changes I cant reproduce the issue anymore, I've tried editing the settings.db and even deleted the testing version. Any clue what I did wrong ?

real-yfprojects · 2023-03-13T13:20:05Z

the UI is not freezing now, but after reverting back to the old changes I cant reproduce the issue anymore

I don't know what you were experiencing before, but this looks good. What happens when you close the dialog, the main window, quit vorta or try to edit anything inside vorta?

DaffyTheDuck · 2023-03-13T17:45:14Z

the UI is not freezing now, but after reverting back to the old changes I cant reproduce the issue anymore

I don't know what you were experiencing before, but this looks good. What happens when you close the dialog, the main window, quit vorta or try to edit anything inside vorta?

Hi! Actually that was my mistake 😓 , I was running vorta from the backup repo and reverted the changes in main repo 😓

But is this the required solution ? like are there more changes required ?

real-yfprojects · 2023-03-14T08:13:43Z

But is this the required solution ? like are there more changes required ?

As I said. This is the right approach. What happens when you close the dialog, the main window or quit vorta?

DaffyTheDuck · 2023-03-14T17:16:13Z

Hi! this seems to work on my machine! waiting for your review 😄

also I've had a question about migrations, how can I perform the migrations 🤔

src/vorta/views/repo_add_dialog.py

real-yfprojects · 2023-03-14T19:55:05Z

@m3nu What was your Intention with the PR, would you consider it fine when the GUI stays responsive but you still can't do anything else while borg is running?

m3nu · 2023-03-14T21:03:28Z

This code was written long ago. It didn't seem important to do, anything else, while adding a repo. No specific consideration.

DaffyTheDuck · 2023-03-15T13:34:51Z

This code was written long ago. It didn't seem important to do, anything else, while adding a repo. No specific consideration.

But as per the current GUI, you can not do anything until you close the dialog displayed 🤔

real-yfprojects · 2023-03-15T14:09:08Z

also I've had a question about migrations, how can I perform the migrations thinking

Which migrations? Usually vorta migrates its database automatically.

DaffyTheDuck · 2023-03-15T14:46:16Z

also I've had a question about migrations, how can I perform the migrations thinking

Which migrations? Usually vorta migrates its database automatically.

Oh, okay, means whenever I run vorta it applies the migrations ?

real-yfprojects · 2023-03-15T15:09:53Z

Yes, if needed.

src/vorta/views/repo_add_dialog.py

DaffyTheDuck · 2023-03-15T17:39:03Z

Is this right ?

src/vorta/views/repo_add_dialog.py

real-yfprojects

Is this right?

Besides some minor problems this is right.

real-yfprojects · 2023-03-17T14:25:24Z

src/vorta/views/repo_add_dialog.py

-                thread.result.connect(self.run_result)
-                self.thread = thread  # Needs to be connected to self for tests to work.
-                self.thread.run()
+                job = BorgInfoRepoJob(params['cmd'], params)


This will be queued in the default queue rather than the repo specific queue.

But I guess the BorgInfoRepoJob is used to validate existing repo right ?

Yes, that's right. You have to pass an extra argument iirc.

Yes, that's right. You have to pass an extra argument iirc.

Hi! sorry for being inactive, I was sick 😅

I see the BorgJob accepting 3 params, 2 are already there, the third one should be sending value default by default since it is not provided 🤔 I was wondering how it's deciding about queue (this question might sound dumb but 😅 )

is this the default value we are talking about 🤔 , because jobs like backup are added to 1 I guess

See this line:

vorta/src/vorta/application.py

Line 113 in 1f12782

job = BorgCreateJob(msg['cmd'], msg, profile.repo.id)

Hi! added the code, but is there any way to verify it that it is going in default queue since I'm unsure it's working properly :)

real-yfprojects · 2023-03-17T14:26:42Z

src/vorta/views/repo_add_dialog.py

@@ -211,6 +214,11 @@ def password_listener(self):
            self.passwordLabel.setText(translate('utils', msg))
            return not bool(msg)

+    def cancel_job(self):
+        if self.job:
+            self.job.cancel()


Sadly this only cancelt the job if its already running. If the job manager hasn't picked up the job yet, that is the job still sits in the queue, it won't be cancelled.

Hi! I'm a bit confused testing this, any suggestion on how can I test this ?

Currently there is no way to reproduce this with the GUI. So you can decide not to deal with it but instead write a code comment on the issue.

Currently there is no way to reproduce this with the GUI. So you can decide not to deal with it but instead write a code comment on the issue.

The code will be included in cancel_job method since it will check for both, if running or in queue, should I create a duplicate method for that and comment it ?

Hi! @real-yfprojects , Since this is not reproducible, I'm not sure that the code will work or not, then is it okay to push the code even it is commented ?

I can only decide after seeing the code.

real-yfprojects · 2023-03-21T08:42:59Z

I noticed that the cancel button no longer works.

DaffyTheDuck · 2023-03-21T09:19:08Z

I noticed that the cancel button no longer works.

when we try to cancel (close) the add repo dialog without hitting the OK? I noticed that too, I added a try-except block for that in cancel_job 👍🏼 I'll push the code once the default queue change is done 😄

src/vorta/views/repo_add_dialog.py

DaffyTheDuck · 2023-04-07T06:50:42Z

I noticed that the cancel button no longer works.

The cancel button should work now 👍🏼

real-yfprojects

You are n't setting a meaningful status message yet which was requested in #1306.

real-yfprojects · 2023-04-08T09:21:00Z

src/vorta/views/repo_add_dialog.py

+            if self.job:
+                self.job.cancel()


There is no self.job currently.

There is no self.job currently.

Hm! The close() method is working!, but I also noticed that you have to cancel the job within a specific time gap it might vary from machine to machine i guess, but after that time gap it adds the repo anyways

You are n't setting a meaningful status message yet which was requested in #1306.

Done 👍🏼

src/vorta/views/repo_add_dialog.py

- Avoids freezing UI when an existing repo is added Closes borgbase#1306

DaffyTheDuck · 2023-04-21T17:50:47Z

Hi! and sorry I was away leaving this PR open with no activity, I was actually having my Practical exams 😅

I've updated the code accordingly, waiting for your review @real-yfprojects 😄

real-yfprojects · 2023-04-21T18:09:59Z

Pls also show a status message while borg init/borg info is running.

DaffyTheDuck · 2023-04-21T19:15:38Z

Pls also show a status message while borg init/borg info is running.

Sorry! did not get that exactly

DaffyTheDuck · 2023-04-22T12:06:03Z

Pls also show a status message while borg init/borg info is running.

Hi! Did you mean, while the app is backing up the data? l instead of messages like Received SIGTERM or ERROR: Connection closed by remote host it should show Cancelled Adding Repository ? Because it shows the Cancelled adding repository message in AddRepoDialog 🤔

real-yfprojects · 2023-04-22T15:31:16Z

No it should show a message while the repository is accessed (verified) for the first time. Something like Adding repository or something more specific and preferably progress updates if available.

DaffyTheDuck · 2023-04-30T02:46:18Z

Hi! @real-yfprojects, sorry, this is taking longer than expected, I'm completely engrossed in preparation of my upcoming semester exam from May 8th. I will be completing this in a slow pace till then.

Sorry for inconvenience :)

real-yfprojects · 2023-04-30T05:31:11Z

No worries.

DaffyTheDuck changed the title ~~[Enhancement] Better status message when adding existing repo~~ [Enhancement] Avoid blocking UI while adding existing repo Mar 13, 2023

DaffyTheDuck force-pushed the issue/1306 branch 2 times, most recently from 005e97f to b250143 Compare March 14, 2023 17:13

real-yfprojects reviewed Mar 14, 2023

View reviewed changes

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

DaffyTheDuck force-pushed the issue/1306 branch 2 times, most recently from 96f9671 to 85e9d09 Compare March 15, 2023 17:36

DaffyTheDuck commented Mar 15, 2023

View reviewed changes

src/vorta/views/repo_add_dialog.py Show resolved Hide resolved

DaffyTheDuck requested a review from real-yfprojects March 16, 2023 14:06

real-yfprojects reviewed Mar 17, 2023

View reviewed changes

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

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

real-yfprojects reviewed Mar 17, 2023

View reviewed changes

DaffyTheDuck force-pushed the issue/1306 branch from 85e9d09 to 6a97953 Compare April 7, 2023 06:45

DaffyTheDuck commented Apr 7, 2023

View reviewed changes

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

DaffyTheDuck commented Apr 7, 2023

View reviewed changes

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

DaffyTheDuck commented Apr 7, 2023

View reviewed changes

src/vorta/views/repo_add_dialog.py Show resolved Hide resolved

DaffyTheDuck force-pushed the issue/1306 branch from 6a97953 to c234fca Compare April 7, 2023 06:50

DaffyTheDuck requested a review from real-yfprojects April 7, 2023 06:50

real-yfprojects reviewed Apr 8, 2023

View reviewed changes

DaffyTheDuck force-pushed the issue/1306 branch from c234fca to 7edd15f Compare April 21, 2023 17:48

[Enhancement] Better status message when adding existing repo

9ac38a1

- Avoids freezing UI when an existing repo is added Closes borgbase#1306

DaffyTheDuck force-pushed the issue/1306 branch from 7edd15f to 9ac38a1 Compare April 21, 2023 17:48

DaffyTheDuck requested a review from real-yfprojects April 21, 2023 17:51

DaffyTheDuck marked this pull request as draft June 15, 2023 09:46

real-yfprojects added the help wanted This issue is available, comment if you want to fix it label Sep 4, 2023

[Enhancement] Avoid blocking UI while adding existing repo #1643

Are you sure you want to change the base?

[Enhancement] Avoid blocking UI while adding existing repo #1643

Conversation

DaffyTheDuck commented Mar 12, 2023

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

DaffyTheDuck commented Mar 12, 2023

real-yfprojects commented Mar 13, 2023

DaffyTheDuck commented Mar 13, 2023

real-yfprojects commented Mar 14, 2023

DaffyTheDuck commented Mar 14, 2023

real-yfprojects commented Mar 14, 2023

m3nu commented Mar 14, 2023

DaffyTheDuck commented Mar 15, 2023

real-yfprojects commented Mar 15, 2023

DaffyTheDuck commented Mar 15, 2023

real-yfprojects commented Mar 15, 2023

DaffyTheDuck commented Mar 15, 2023

real-yfprojects left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DaffyTheDuck Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

real-yfprojects commented Mar 21, 2023

DaffyTheDuck commented Mar 21, 2023

DaffyTheDuck commented Apr 7, 2023

real-yfprojects left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DaffyTheDuck Apr 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DaffyTheDuck commented Apr 21, 2023

real-yfprojects commented Apr 21, 2023

DaffyTheDuck commented Apr 21, 2023

DaffyTheDuck commented Apr 22, 2023 • edited Loading

real-yfprojects commented Apr 22, 2023 • edited Loading

DaffyTheDuck commented Apr 30, 2023

real-yfprojects commented Apr 30, 2023

DaffyTheDuck Mar 21, 2023 •

edited

Loading

DaffyTheDuck Apr 8, 2023 •

edited

Loading

DaffyTheDuck commented Apr 22, 2023 •

edited

Loading

real-yfprojects commented Apr 22, 2023 •

edited

Loading