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

non interactive updater #199

Merged
merged 21 commits into from
Jun 28, 2024

Conversation

piotrbartman
Copy link
Member

This PR is a general solution to several issues, particularly related to the use of CLI arguments.
It implements the return of non-zero codes when an update fails, is canceled, or when no new updates are available.
CLI arguments have been unified with the backend to make their usage easier.
The reason for this PR is distinguishing between situations when we can confirm that the system is fully updated and when it is only partially updated.

depends on QubesOS/qubes-core-admin-linux/pull/157

fixes QubesOS/qubes-issues#9014
fixes QubesOS/qubes-issues#9032
fixes QubesOS/qubes-issues#9225

Copy link

codecov bot commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 94.21769% with 17 lines in your changes missing coverage. Please review.

Project coverage is 93.53%. Comparing base (8bf07aa) to head (15dc1f1).
Report is 2 commits behind head on main.

Files Patch % Lines
qui/updater/updater.py 83.90% 14 Missing ⚠️
qui/updater/summary_page.py 88.23% 2 Missing ⚠️
qui/updater/progress_page.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #199      +/-   ##
==========================================
+ Coverage   93.20%   93.53%   +0.32%     
==========================================
  Files          57       57              
  Lines       10601    10816     +215     
==========================================
+ Hits         9881    10117     +236     
+ Misses        720      699      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@deeplow deeplow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the PR and fantastic work! This non-interactive approach quite elegantly tackles all the outstanding issues I had filed.

The only other thing that stood out to me was that when using --apply-to-all the user gets two messages to acknowledge:

  • ✔️ All qubes were restarted / shutdown successfully [OK]
  • ✔️ All selected qubes have been updated [OK]

Any way these could be turned into one single message so the user doesn't have to acknowledge two prompts?

'(default: %(default)d)',
type=int, default=default_update_if_stale)
update_state.add_argument(
'--update-if-available', action='store_true',
Copy link

@deeplow deeplow Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing this argument is erroring for me. I am running qubes-updater-gui --targets fedora-40-xfce --update-if-available but it's failing with a traceback and the following message:

qubes-vm-update: error: unrecognized arguments: --update-if-available

I think what's going on here is that the GUI in fact applying the necessary modification in the HeaderCheckbox, but it is still trying to pass that argument to qubes-vm-update instead of ignoring it.

Copy link
Member Author

@piotrbartman piotrbartman Jun 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably because you didn't update qubes-vm-update, see

"depends on QubesOS/qubes-core-admin-linux#157"

The PR is ready to install, only ci tests do not work.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. I see. I missed that. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants