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

Update label displaying next backup time. #1180

Merged
merged 2 commits into from
Feb 9, 2022

Conversation

real-yfprojects
Copy link
Collaborator

Always update the label in the schedule tab that displays the next time to start a backup when the scheduler updates its timer.
Fixes #1116.

  • src/vorta/scheduler.py (VortaScheduler): Add schedule_changed signal.

  • src/vorta/scheduler.py (VortaScheduler.set_timer_for_profile): Emit schedule_changed signal.

  • src/vorta/views/schedule_tab.py (ScheduleTab): Connect draw_next_scheduled_backup to schedule_changed signal.

@real-yfprojects
Copy link
Collaborator Author

I was able to track the error down:

When VortaApp is initialized it creates a main window which in turn creates a ScheduleTab connecting to the scheduler. In the tests (conftest.init_db) the main window is deleted while the schedule tab is not while remaining connected to the signal. This instance will then produce the error seen because it isn't able to access its parent window.

This is clearly an issue with the testing setup or maybe with VortaApp creating the main window. Could you have a look at the configuration of the test and try to fix this, @m3nu?

@m3nu
Copy link
Contributor

m3nu commented Jan 29, 2022

Redoing all the tests sounds like a big task just to preview the next scheduler run more often.

I keep the app around, but reset the database and windows during tests. Destroying the whole app would take much longer and caused segfaults from Qt, as it expects only one Qt app per process. 😬

@real-yfprojects real-yfprojects force-pushed the update-backup-time-label-1116 branch 3 times, most recently from 625b42b to 1ed23be Compare January 29, 2022 15:18
@real-yfprojects
Copy link
Collaborator Author

I implemented a work-around for the failing test.

@m3nu
Copy link
Contributor

m3nu commented Feb 3, 2022

That's a clever implementation and good workaround for the test. Did something like this myself in a few places. Good job!

Let me run this locally for a while and then merge. Thanks!

@m3nu m3nu self-assigned this Feb 3, 2022
@real-yfprojects
Copy link
Collaborator Author

Do you know that you don't have to merge the master into the feature branch if you merge it into master afterwards? In case you want to check the tests you can also rebase the branch from within the github ui or using git rebase master. That won't add a new commit to the history.

…imer.

Fixes borgbase#1116.

* src/vorta/scheduler.py (VortaScheduler): Add `schedule_changed` signal.

* src/vorta/scheduler.py (VortaScheduler.set_timer_for_profile): Emit `schedule_changed` signal.

* src/vorta/views/schedule_tab.py (ScheduleTab): Connect `draw_next_scheduled_backup` to `schedule_changed` signal.
* tests/test_schedule.py : Work around old scheduletabs still being connected to the
	scheduler's signal by reconnecting the scheduletab.
@m3nu
Copy link
Contributor

m3nu commented Feb 9, 2022

Works as promised. Very elegant solution. Thanks for taking care! 💃

@m3nu m3nu merged commit de971d6 into borgbase:master Feb 9, 2022
@real-yfprojects real-yfprojects deleted the update-backup-time-label-1116 branch February 9, 2022 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Next Backup time under the schedule tab is never updated
2 participants