action #63883
closedopenqa-clone-job does not support removing an unuseful setting
0%
Description
User story¶
Sometimes users need to remove some settings when trying to clone a new job. For now, if users do not want some setting make sense, they will set a invalid value to those setting.
Acceptance criteria¶
- when executing openqa-clone-job, users could remove some settting by set KEY= or --delete_settings KEY1,KEY2,...
Updated by okurz about 4 years ago
- Target version set to future
I know this is based on a chat with mkittler but I am not sure what the real problem was because setting a value to an empty value works with e.g. FOO=. Maybe the problem was only when using --parental-inheritance?
Updated by Xiaojing_liu about 4 years ago
okurz wrote:
I know this is based on a chat with mkittler but I am not sure what the real problem was because setting a value to an empty value works with e.g.
FOO=. Maybe the problem was only when using--parental-inheritance?
I did a test again and look up the code. When we set FOO=, the setting FOO will be really deleted by this function clone_job_apply_settings. But when creating a new job, the create function will generate the setting again. Maybe the create function in API/V1/Job.pm should simply create a new job using the parameters when users clone jobs. This is also the question that mentioned in #63565.
Updated by Xiaojing_liu about 4 years ago
- Related to action #63565: The extra setting is added to the new job when cloning a job added
Updated by livdywan about 4 years ago
- Status changed from New to In Progress
- Assignee set to Xiaojing_liu
- Target version changed from future to Current Sprint
Xiaojing_liu wrote:
I did a test again and look up the code. When we set
FOO=, the settingFOOwill be really deleted by this functionclone_job_apply_settings. But when creating a new job, thecreatefunction will generate the setting again. Maybe thecreatefunction in API/V1/Job.pm should simply create a new job using the parameters when users clone jobs. This is also the question that mentioned in #63565.
While this isn't a concrete bug I second Jane this should have been consistent to begin with. The special-case of an empty value is missing here because it's two code paths.
Updated by Xiaojing_liu about 4 years ago
Updated by okurz about 4 years ago
Had to revert https://github.com/os-autoinst/openQA/pull/2931 with https://github.com/os-autoinst/openQA/pull/2950 due to #65750
Updated by okurz about 4 years ago
- Related to action #65750: [17/04/2020 07:56:17] <DimStar> okurz: Martchus_ would appreciate if you could gave a look at tw snapshot 0415... dozens of incomplete tests (attempting to retrigger comolains about missing assets) added
Updated by livdywan about 4 years ago
- Status changed from Feedback to In Progress
Semi-related, os-autoinst/openQA#2931 which was inspired by this, in fact uncovered an apparent bug (in existing unit tests) as well as a yet to be investigated bug (in Tumbleweed Snapshot tests, see #65750).
Updated by livdywan almost 4 years ago
Updated by Xiaojing_liu almost 4 years ago
- Status changed from In Progress to Feedback
PR has been merged
Updated by acarvajal almost 4 years ago
Hello,
I believe https://github.com/os-autoinst/openQA/pull/2959 introduced a regression that makes openQA perform the replacement of placeholders in settings before ensuring the value of DISTRI is lower case. This caused some tests to fail to find their qcow2 images referenced by HDD_1 which in turn has a %DISTRI% as part of its value.
For example: https://openqa.suse.de/tests/4189650
Last good: https://openqa.suse.de/tests/4174255
Not aware of other problems, but any setting with %DISTRI% must have been affected.
Call to OpenQA::JobSettings::expand_placeholders(\%settings); was originally after $settings{DISTRI} = lc($settings{DISTRI}); in L548 of OpenQA::Schema::Result::ScheduleProducts, but now this call is in OpenQA::JobSettings::generate_settins(\%params); which is called before.
Updated by livdywan almost 4 years ago
acarvajal wrote:
For example: https://openqa.suse.de/tests/4189650
Last good: https://openqa.suse.de/tests/4174255Not aware of other problems, but any setting with
%DISTRI%must have been affected.Call to
OpenQA::JobSettings::expand_placeholders(\%settings);was originally after$settings{DISTRI} = lc($settings{DISTRI});in L548 ofOpenQA::Schema::Result::ScheduleProducts, but now this call is inOpenQA::JobSettings::generate_settins(\%params);which is called before.
I think os-autoinst/openQA#3045 should be addressing that? It was merged on Thursday
Updated by Xiaojing_liu almost 4 years ago
cdywan wrote:
acarvajal wrote:
For example: https://openqa.suse.de/tests/4189650
Last good: https://openqa.suse.de/tests/4174255Not aware of other problems, but any setting with
%DISTRI%must have been affected.Call to
OpenQA::JobSettings::expand_placeholders(\%settings);was originally after$settings{DISTRI} = lc($settings{DISTRI});in L548 ofOpenQA::Schema::Result::ScheduleProducts, but now this call is inOpenQA::JobSettings::generate_settins(\%params);which is called before.I think os-autoinst/openQA#3045 should be addressing that? It was merged on Thursday
yes, #3045 is used to fix that.
Updated by acarvajal almost 4 years ago
cdywan wrote:
I think os-autoinst/openQA#3045 should be addressing that? It was merged on Thursday
Checking at some jobs for settings that use %DISTRI%, problem does seem fixed. Thanks a lot.
Updated by Xiaojing_liu almost 4 years ago
- Status changed from Feedback to Resolved
- Target version deleted (
Current Sprint)