action #164712
closedRemove the deprecated "script_run($cmd, die_on_timeout => 0)" and handle follow-up size:M
Description
Motivation¶
See #164541. script_run($cmd, die_on_timeout => 0)
was declared deprecated some years ago. We should follow up by obsoleting and helping affected users (or revert the deprecation).
Acceptance criteria¶
- AC1: The use case of starting a script, doing something else and finally waiting for the script to finish is still covered.
- AC2: The use case of starting a script and handling a stall or timeout gracefully is still covered
- AC3: No deprecated modes of
script_run
persist intestapi.pm
Suggestions¶
- Review past decisions
- Provide a change to osado to not have
script_run($cmd, die_on_timeout => 0)
- For AC2 it's suggested to use
enter_cmd && wait_serial
(which should be documented, e.g. a hint on theenter_cmd
documentation explaining the possibility to chain withwait_serial
would make sense)
Updated by okurz 5 months ago
Asked in #eng-testing https://suse.slack.com/archives/C02CANHLANP/p1722356418947619
@Fabian Vogt In https://github.com/os-autoinst/os-autoinst/pull/1807 you stated that you don't see a valid use case for script_run($cmd, die_on_timeout => 0). In light of https://suse.slack.com/archives/C02CGKV748P/p1722225117322959 where people are concerned about the deprecation I wonder if we can find good replacements for the mentioned code like https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/lib/wickedbase.pm#L1264 or if we should revert the deprecation. WDYT?
Updated by livdywan 4 months ago
okurz wrote in #note-2:
https://github.com/os-autoinst/os-autoinst/pull/2520
And
https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19853
First branch merged, second branch under review by stakeholders
Updated by okurz 4 months ago
- Due date changed from 2024-08-14 to 2024-09-18
merged https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/19853
now https://github.com/os-autoinst/os-autoinst/pull/2521 marked as ready for review but with label "not-ready" to give enough time for preparation until merge.
Updated by livdywan 4 months ago
https://suse.slack.com/archives/C02CANHLANP/p1723798592289769 announcement on Slack
Updated by okurz 3 months ago
- Due date changed from 2024-09-18 to 2024-10-02
I reminded the team again to approve https://github.com/os-autoinst/os-autoinst/pull/2521
Updated by okurz 3 months ago
- Due date deleted (
2024-10-02) - Status changed from Feedback to Resolved
https://github.com/os-autoinst/os-autoinst/pull/2521 also now active on OSD since some days. No negative feedback received.