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

Extract backend code to it's own package #6316

Merged
merged 6 commits into from Dec 14, 2018

Conversation

foursixnine
Copy link
Member

@foursixnine foursixnine commented Nov 29, 2018

As stated in poo#33388, we have code that is being used across diferent modules (use_ssh_serial_console in this case), and that should have it's own module..

There are many other ocurrences, but for the time being let's introduce Utils::Backends and then start to grow from there

I'm still pending to have this running on spvm, ipmi and svirt backends, just to make sure that stuff is not broken

Check poo#45008 which should be a continuation of this one...

@foursixnine foursixnine changed the title Replace uses of is_remote_backend Tidy up Add Utils::Backends Move use_ssh_serial_console to Utils::Backends Remove use_ssh_serial_console Remove is_remote_backend [WIP] Split backend code Nov 29, 2018
@foursixnine foursixnine added notready WIP Work in progress labels Nov 29, 2018
lib/Utils/Backends.pm Outdated Show resolved Hide resolved
lib/Utils/Backends.pm Outdated Show resolved Hide resolved
lib/Utils/Backends.pm Outdated Show resolved Hide resolved
lib/main_common.pm Show resolved Hide resolved
lib/version_utils.pm Show resolved Hide resolved
tests/autoyast/login.pm Outdated Show resolved Hide resolved
tests/autoyast/login.pm Outdated Show resolved Hide resolved
@foursixnine foursixnine changed the title [WIP] Split backend code Extract backend code to it's own package Dec 11, 2018
@foursixnine foursixnine removed WIP Work in progress notready labels Dec 11, 2018
@foursixnine
Copy link
Member Author

@okurz @rwx788 You guys have any more comments?

@rwx788
Copy link
Member

rwx788 commented Dec 13, 2018

@foursixnine I need to take another look as you've done more work. Will try to do so ;)

],
CONSOLES => [
qw(
use_ssh_serial_console
Copy link
Member

Choose a reason for hiding this comment

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

I know that you are addressing another issue, but I wanted to mention that is a hack. And then we have a lot of calls like check_var('BACKEND', 'ipmi') ? use_ssh_serial_console : select_console 'root-console';.
Last time I tried to fix select_console with callbacks, but could not finish it quick, so postponed the idea. However, I believe we should get rid of this method in all 15 files and have select_console "root-console" only. Same for user-console, except that we should switch user.

Copy link
Member Author

Choose a reason for hiding this comment

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

@rwx788 actually that's a path I'd choose for later. A bit more care needs to be added there, But completely feasible, specially if poo#45146 is addressed properly and we stop covering at the test distri level, for things that should be provided by the backend...

I'm actually cringing after seeing stuff like this :)

Copy link
Member

@rwx788 rwx788 left a comment

Choose a reason for hiding this comment

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

LGTM, just mentioned follow-up idea which would help us to execute more tests on ipmi, as of now they won't work.

@foursixnine
Copy link
Member Author

@rwx788 thanks! I'll merge later if @okurz doesn't have any other comments

@foursixnine foursixnine merged commit 83fb269 into os-autoinst:master Dec 14, 2018
@foursixnine foursixnine deleted the split_backends branch December 14, 2018 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants