-
Notifications
You must be signed in to change notification settings - Fork 354
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
Make possible to start TUI with installed WebUI #5295
Make possible to start TUI with installed WebUI #5295
Conversation
Hello @adamkankovsky! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2023-11-20 14:50:47 UTC |
f3a7af2
to
378aad5
Compare
378aad5
to
615771f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice and clean. I like it! One note, I am pretty sure that starting Web UI in an installation environment without GUI will cause issues. In the previous version, we won't start Xorg if necessary. In your version, I guess the installer will switch to the text mode. Here is the problematic code:
anaconda/pyanaconda/display.py
Lines 305 to 311 in 93b3967
if anaconda.gui_mode: | |
mods = (tup[1] for tup in pkgutil.iter_modules(pyanaconda.ui.__path__, "pyanaconda.ui.")) | |
if "pyanaconda.ui.gui" not in mods: | |
stdout_log.warning("Graphical user interface not available, falling back to text mode") | |
anaconda.display_mode = constants.DisplayModes.TUI | |
flags.usevnc = False | |
flags.vncquestion = False |
@jkonecny12 That's why every installation image needs to be properly tested if you want to have multiple images with different contents. It is easy to break the installer by removing a package from the installation environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, mostly LGTM. What Vendy said - please keep the condition flow "one direction and one level".
98f20f8
to
46e4fc4
Compare
46e4fc4
to
60f5718
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great! Thank you!
/kickstart-test --testtype smoke |
Trying to keep only inst.graphical in the future, instead of inst.webui, because webui as such should not be represented in KickStart.