Project

General

Profile

Actions

action #164712

closed

Remove the deprecated "script_run($cmd, die_on_timeout => 0)" and handle follow-up size:M

Added by okurz 5 months ago. Updated 3 months ago.

Status:
Resolved
Priority:
Low
Assignee:
Category:
Feature requests
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:

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 in testapi.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 the enter_cmd documentation explaining the possibility to chain with wait_serial would make sense)
Actions #1

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?

Actions #3

Updated by okurz 5 months ago

  • Subject changed from Handle deprecation of "script_run($cmd, die_on_timeout => 0)" to Remove the deprecated "script_run($cmd, die_on_timeout => 0)" and handle follow-up size:M
  • Description updated (diff)
Actions #4

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

Actions #5

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.

Actions #7

Updated by okurz 4 months ago

  • Priority changed from Normal to Low
Actions #8

Updated by livdywan 4 months ago

This might actually be a use case for #136127 as it seems the expectations on the script_run API were not clear.

Actions #9

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

Actions #11

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.

Actions

Also available in: Atom PDF