Skip to content
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

[Enhancement] Added setting to run vorta in background #1620

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DaffyTheDuck
Copy link

@DaffyTheDuck DaffyTheDuck commented Mar 1, 2023

Description

When closing vorta a popup will be displayed whether to continue running in the background or not, according to users response the application will close or will stay in the system tray

Related Issue

Closes #1582

Motivation and Context

Gives the user control of the background activity (whether he/she wants to continue the process) through the checkbox added in the mics tab

How Has This Been Tested?

Machine: Ubuntu 20.04
Python Version: 3.10.6
Borg Version: 1.2.0

I've tested this on my local environment installed by following dev guidelines mentioned here

Screenshots (if appropriate):

  1. Added Background Group Checkbox
    image

  2. When Checked Asks For The Following Option
    image

  3. If Checked Don Not Show This Again The Background Question Checkbox Unchecks (Big Thanks To @real-yfprojects)
    image

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:

  • 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 read the CONTRIBUTING guide.
  • 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
Copy link
Author

Hi!

  1. Actually the previous commit got discarded when I rebased the content with my fork with the old changes still with me (I'm still figuring it out how this happened)
  2. I've not checked the All new and existing tests passed. since on my machine the pytests are failing (I guess this change is non-breaking 🤔 ), I've tested the changes which are working

Since this is the first time with pytests I would love to hear suggestions from Mentors 😄

@DaffyTheDuck DaffyTheDuck changed the title - Added setting to run vorta in background [Enhancement] Added setting to run vorta in background Mar 1, 2023
src/vorta/views/misc_tab.py Outdated Show resolved Hide resolved
src/vorta/views/misc_tab.py Show resolved Hide resolved
src/vorta/views/misc_tab.py Show resolved Hide resolved
@DaffyTheDuck DaffyTheDuck force-pushed the feature/bg-setting branch 2 times, most recently from bd50c83 to b68c421 Compare March 3, 2023 07:47
@DaffyTheDuck
Copy link
Author

Hi! Just out of curiosity! Why do the tests take time to report? like when I worked with an organization previously there they used a ci tool called Travis where you could see the tests running and it used to report the test results within minutes 😃

@m3nu
Copy link
Contributor

m3nu commented Mar 3, 2023

Because the tests won't run automatically for new contributors and need manual approval first.

After your first contribution, they will run automatically for you and will finish in about 1-2 min.

@DaffyTheDuck
Copy link
Author

Because the tests won't run automatically for new contributors and need manual approval first.

After your first contribution, they will run automatically for you and will finish in about 1-2 min.

Oh informative 😄

@real-yfprojects
Copy link
Collaborator

The tests fail due to a different issue now: peewee doesn't support connecting multiple methods with the same name. You will have to disconnect the signal after each test. peewee docs

@DaffyTheDuck
Copy link
Author

DaffyTheDuck commented Mar 3, 2023

The tests fail due to a different issue now: peewee doesn't support connecting multiple methods with the same name. You will have to disconnect the signal after each test. peewee docs

Hi! I've added the disconnect method mentioned in docs, which results in 2 test failures (autostart and test_create) because it exceeds time. Is something wrong on my side (like my approach is wrong or something?) because all other tests passes but these two fails 🤔

@real-yfprojects
Copy link
Collaborator

I can't tell you because you didn't push your changes.

@DaffyTheDuck
Copy link
Author

You will have to disconnect the signal after each test. peewee docs

Hi! I could not get this actually, I'm figuring out where to call the disconnect mentioned in docs, do I have to modify the tests ? because I tried putting it in mics tab when the checkbox is updated then disconnect but it results in a small bug 😅

@real-yfprojects
Copy link
Collaborator

No, you have to add it to the test setup. There is a fixture in conf.py.

@DaffyTheDuck
Copy link
Author

Hi! There's still 1 test which fails 😓

tests/conftest.py Outdated Show resolved Hide resolved
@DaffyTheDuck DaffyTheDuck force-pushed the feature/bg-setting branch 4 times, most recently from c6f648d to a0d5a46 Compare March 7, 2023 06:35
@DaffyTheDuck DaffyTheDuck force-pushed the feature/bg-setting branch 2 times, most recently from 8bc980d to 96edb9f Compare March 7, 2023 14:46
@bigtedde
Copy link
Collaborator

@DaffyTheDuck @m3nu @real-yfprojects what do you think about implementing a setting for running the application in the background directly, instead of a setting to prompt the user each time they close the window?

@DaffyTheDuck
Copy link
Author

@DaffyTheDuck @m3nu @real-yfprojects what do you think about implementing a setting for running the application in the background directly, instead of a setting to prompt the user each time they close the window?

It's not actually prompting each time it's upon the user, he can choose to show or hide the dialog, I opened this PR because I thought some users might be using Manual Backup all the time (like me) so for them keeping the application running in background might not be an option, and in current version, if you chose the option don't show this again on the dialog and close the application there's no option to bring that dialog back in mics tab (it might be elsewhere but from a prospective of a new user he'll be looking into mics tab for that since most of the settings are there)

There are other cases too, like once you do the manual backup and on other day you want a scheduled backup for some reason the application will keep closing since you chose the don't show this again and No as option in that "Keep In Background Dialog"

@real-yfprojects
Copy link
Collaborator

what do you think about implementing a setting for running the application

I wanted to add this option additionally to the changes of this PR. I decided to open an issue for that once this is merged.

@real-yfprojects
Copy link
Collaborator

This has to be rebased onto master and upgraded to PyQt6 @DaffyTheDuck. Maybe @i1sm3ky can guide you through that.

@i1sm3ky
Copy link
Contributor

i1sm3ky commented Apr 29, 2023

This has to be rebased onto master and upgraded to PyQt6 @DaffyTheDuck. Maybe @i1sm3ky can guide you through that.

Yes, I'm more than happy to help. Just LMK if you face any issues while upgrading or want to know where to start!

@DaffyTheDuck
Copy link
Author

This has to be rebased onto master and upgraded to PyQt6 @DaffyTheDuck. Maybe @i1sm3ky can guide you through that.

Yes, I'm more than happy to help. Just LMK if you face any issues while upgrading or want to know where to start!

Sure 😄

@m3nu
Copy link
Contributor

m3nu commented Jun 9, 2023

Still planning to rebase this to Qt6, @DaffyTheDuck ?

@m3nu m3nu marked this pull request as draft June 9, 2023 11:38
@DaffyTheDuck
Copy link
Author

Still planning to rebase this to Qt6, @DaffyTheDuck ?

Hi! Yes! I was just freed the day before yesterday, I'll start with this again from tomorrow 😄

@DaffyTheDuck
Copy link
Author

Hi! @real-yfprojects , I had some doubts about the conflicts in conftest.py is the peewee issue which was faced during the initial stage solved ? If I recall correctly you raised an issue in the official repo. Now actually I'm unable to find it 😅

@real-yfprojects
Copy link
Collaborator

real-yfprojects commented Jun 12, 2023

Its coleifer/peewee#2687
Since its fixed we can either pin peewee to the version it was fixed in and remove the workaround

@DaffyTheDuck
Copy link
Author

Its coleifer/peewee#2687 Since its fixed we can either pin peewee to the version it was fixed in and remove the workaround

Done with the conflict locally! But before pushing I want to test it :) . Running the dev version produces the following error

qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it was found.
This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem.

Available platform plugins are: eglfs, vnc, offscreen, minimalegl, xcb, vkkhrdisplay, linuxfb, wayland-egl, wayland, minimal.

[1]    9013 IOT instruction (core dumped)  vorta

I've updated every requirement since it was inactive for more than a month, am I doing something wrong ?

@m3nu
Copy link
Contributor

m3nu commented Jun 14, 2023

How did you install PyQt? From distro packages or from PyPi? This error is often related to that. Or maybe the Python env has an old PyQt version? Maybe start over with the virtual env? Also, you're running this with a window manager? It won't work on a headless system.

@DaffyTheDuck
Copy link
Author

How did you install PyQt? From distro packages or from PyPi? This error is often related to that. Or maybe the Python env has an old PyQt version? Maybe start over with the virtual env? Also, you're running this with a window manager? It won't work on a headless system.

Hm, still giving the same message, I'll change the windows manager to test again, should I solve the conflicts here using github ?

@m3nu
Copy link
Contributor

m3nu commented Jun 14, 2023

The Github interface is pretty basic. There are better local tools. And it's better practice to do it locally. Just be careful not do add any unintentional changes while resolving conflicts.

@DaffyTheDuck DaffyTheDuck force-pushed the feature/bg-setting branch 2 times, most recently from 10fefab to 2cb9afd Compare June 14, 2023 12:40
@DaffyTheDuck
Copy link
Author

DaffyTheDuck commented Jun 14, 2023

The Github interface is pretty basic. There are better local tools. And it's better practice to do it locally. Just be careful not do add any unintentional changes while resolving conflicts.

Hmm! the vscode conflict solving made it worst by discarding my commits, I still have the change on my local machine, any way I can reopen this PR ??

@real-yfprojects
Copy link
Collaborator

Hmm! the vscode conflict solving made it worst by discarding my commits

Then you are doing something wrong.

I still have the change on my local machine, any way I can reopen this PR ??

Just push the local branch to this branch of the fork. Then click the Reopen button here on Github.

Added setting to enable/disable the run in background window

Closes borgbase#1582
@DaffyTheDuck DaffyTheDuck reopened this Jun 15, 2023
@DaffyTheDuck
Copy link
Author

Hmm! the vscode conflict solving made it worst by discarding my commits

Then you are doing something wrong.

I still have the change on my local machine, any way I can reopen this PR ??

Just push the local branch to this branch of the fork. Then click the Reopen button here on Github.

Done! Thank you very much :)

@DaffyTheDuck
Copy link
Author

Hmm! the vscode conflict solving made it worst by discarding my commits

Then you are doing something wrong.

Yes, after searching on internet I understood that I mistakenly pushed the incoming changes to the working branch instead of master then rebased it overwriting the changes 😓

@real-yfprojects real-yfprojects added the help wanted This issue is available, comment if you want to fix it label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted This issue is available, comment if you want to fix it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add setting for running vorta in the backround.
6 participants