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

Render autoinst url from openQA url #21292

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

waynechen55
Copy link
Contributor

@waynechen55 waynechen55 commented Feb 26, 2025

@waynechen55 waynechen55 marked this pull request as draft February 26, 2025 08:09
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

  • Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files.

@waynechen55 waynechen55 force-pushed the wayne/render_autoinst_url branch 9 times, most recently from 1dc24c6 to 1ba68ee Compare March 3, 2025 07:30
@waynechen55 waynechen55 marked this pull request as ready for review March 3, 2025 08:29
@waynechen55 waynechen55 force-pushed the wayne/render_autoinst_url branch from 1ba68ee to dea269b Compare March 3, 2025 09:13
@waynechen55
Copy link
Contributor Author

@alice-suse @guoxuguang @Julie-CAO @RoyCai7 @nanzhg @tbaev Please have a look.

Copy link
Contributor

@alice-suse alice-suse left a comment

Choose a reason for hiding this comment

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

LGTM

@Julie-CAO
Copy link
Contributor

sles15sp7 on sles15sp7

Has this test used the cached Rendered autoinst url? I'd like to see how it is presented but I failed to find the "Record_info" in screenshots?

@waynechen55
Copy link
Contributor Author

sles15sp7 on sles15sp7

Has this test used the cached Rendered autoinst url? I'd like to see how it is presented but I failed to find the "Record_info" in screenshots?

It does not use resoures in assets/hdd

@waynechen55 waynechen55 marked this pull request as draft March 4, 2025 05:10
@waynechen55 waynechen55 force-pushed the wayne/render_autoinst_url branch from dea269b to 6291a23 Compare March 4, 2025 05:20
@Julie-CAO
Copy link
Contributor

sles15sp7 on sles15sp7

Has this test used the cached Rendered autoinst url? I'd like to see how it is presented but I failed to find the "Record_info" in screenshots?

It does not use resoures in assets/hdd

So is it usually applied for tests with SLM guests, or future SLE16 guests, which need .qcow2 files? and nothing to be done on openqa settings for our current tests?

1.New subroutine render_autoinst_url in lib/utils.pm to render
autoinst url from openQA, in order to avoid direct downloading
from openQA instance.
2.Replace direct openQA address with rendered autoinst url in
lib/guest_installation_and_configuration_base.pm.
@waynechen55 waynechen55 force-pushed the wayne/render_autoinst_url branch from 6291a23 to 06b0f1a Compare March 4, 2025 06:43
@waynechen55 waynechen55 marked this pull request as ready for review March 4, 2025 07:01
@waynechen55
Copy link
Contributor Author

sles15sp7 on sles15sp7

Has this test used the cached Rendered autoinst url? I'd like to see how it is presented but I failed to find the "Record_info" in screenshots?

It does not use resoures in assets/hdd

So is it usually applied for tests with SLM guests, or future SLE16 guests, which need .qcow2 files? and nothing to be done on openqa settings for our current tests?

For those using resources assets/.hdd. Nothing to be changed.
Build5.3 also passed:

@Julie-CAO
Copy link
Contributor

sles15sp7 on sles15sp7

Has this test used the cached Rendered autoinst url? I'd like to see how it is presented but I failed to find the "Record_info" in screenshots?

It does not use resoures in assets/hdd

So is it usually applied for tests with SLM guests, or future SLE16 guests, which need .qcow2 files? and nothing to be done on openqa settings for our current tests?

For those using resources assets/.hdd. Nothing to be changed. Build5.3 also passed:

Get it. Thanks for the fix. LGTM

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.

3 participants