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 variant handling in SubiquityServer #2128

Merged

Conversation

Chris-Peterson444
Copy link
Collaborator

Part 1 of 2 for adding support for a "core" variant from #2127.

This PR introduces 1 behavior change and 1 bug fix surrounding variant handling, and 1 function signature fix for a semi-related function in the SubiquityController class.

  • Behavior change: On startup, the server's variant will be set to the variant of the selected source in the catalog. Previously, the source controller would pick the first entry in the source catalog but not update the variant for the server.
  • Bug fix: Fixes a bug where configuring the source controller only updates the SubiquitModel (app.base_model) and not the server's notion of the variant (app.variant). Now selecting a new source will update both.
  • Minor fix: Update SubiquityController.interactive to return False instead of None for non-interactive controllers, since this is always used in truthy contexts.

There is one temporary commit here that adds "core" as a supported variant in subiquity/server/server.py, just so that the examples/sources/core-desktop.yaml answers test will pass. (The TUI will get "core" as the client and then try to post it to the server, which would otherwise fail without this commit.) We can drop this commit before merging, as the logic to add "core" as a supported variant is handled in the other PR.

subiquity/server/controller.py Show resolved Hide resolved
subiquity/server/server.py Outdated Show resolved Hide resolved
subiquitycore/tests/mocks.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@mwhudson mwhudson left a comment

Choose a reason for hiding this comment

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

Looks ok to me. I think we can beef up that docstring a bit though.

subiquity/server/controller.py Show resolved Hide resolved
subiquity/server/server.py Outdated Show resolved Hide resolved
@Chris-Peterson444 Chris-Peterson444 force-pushed the server-variant-handling branch 3 times, most recently from 36b1185 to a8c4e62 Compare January 21, 2025 22:45
Chris-Peterson444 and others added 3 commits January 21, 2025 14:46
Make the return value of `SubiquityController.interactive` an explicit
False return if the controller is not interactive.

Previously, if none of the `if` blocks passed then `interactive` would
implicility return `None`, which would effectively return `False` when
used in `if controller.interactive()` checks. Let's make it explicit.

Additionally, add a docstring that describes the ways in which the _active
value can be changed and generally when controllers get configured.

Lastly, add some tests to the controller class to assert behavior about
interactivity and the interactive_for_variants functionality.

Co-authored-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Fix a mismatch between the variant of the server and the variant of the
SubiquityModel. POST requests to /meta/client_variant would modify the
variant for both the server and the SubiquityModel, while POST requests to
the source controller would update only the SubiquityModel.

Fix this by making all requests to change the variant go through a
single function in the server and let that be responsible for updating
itself and the SubiquityModel.

(Conceptually the Source controller should not be responsible for
updating the SubiquityModel.)
When the source controller starts in the server, it reads the source
catalog and sets the currently selected version to the first item in the
list. Subiquity server starts with a default "server" variant, but needs
to be updated on startup if the install media doesn't match that expectation.
subiquity/server/controller.py Show resolved Hide resolved
@Chris-Peterson444 Chris-Peterson444 merged commit 8a7770a into canonical:main Jan 21, 2025
10 checks passed
@Chris-Peterson444 Chris-Peterson444 deleted the server-variant-handling branch January 21, 2025 23:18
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.

4 participants