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

Make possible to start TUI with installed WebUI #5295

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

adamkankovsky
Copy link
Contributor

Trying to keep only inst.graphical in the future, instead of inst.webui, because webui as such should not be represented in KickStart.

@pep8speaks
Copy link

pep8speaks commented Nov 1, 2023

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

Copy link
Contributor

@poncovka poncovka left a 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:

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.

pyanaconda/anaconda.py Show resolved Hide resolved
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a 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".

pyanaconda/anaconda.py Outdated Show resolved Hide resolved
@adamkankovsky adamkankovsky force-pushed the webui-possible-start-tui branch 4 times, most recently from 98f20f8 to 46e4fc4 Compare November 13, 2023 10:37
pyanaconda/anaconda.py Outdated Show resolved Hide resolved
Copy link
Contributor

@poncovka poncovka left a 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!

@KKoukiou
Copy link
Contributor

/kickstart-test --testtype smoke

@KKoukiou KKoukiou merged commit f4fa15c into rhinstaller:master Nov 21, 2023
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

6 participants