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

Allow scheduling and monitoring a product via openqa-cli #5036

Merged
merged 9 commits into from
Mar 20, 2023

Conversation

Martchus
Copy link
Contributor

@Martchus Martchus commented Mar 14, 2023

Related ticket: https://progress.opensuse.org/issues/125720


Example invocation:

script/openqa-cli schedule --apikey … --apisecret … --host … DISTRI=example VERSION=0 FLAVOR=DVD ARCH=x86_64 TEST=simple_boot FOO=bar --param-file SCENARIO_DEFINITIONS_YAML=/hdd/openqa-devel/openqa/share/tests/example/scenaio-definitions.yaml -r

So one can use CLI-flags similar to openqa-cli api.

Still a draft because unit tests are missing and the error handling needs to be improved (it should retry like openqa-cli api).

@codecov
Copy link

codecov bot commented Mar 15, 2023

Codecov Report

Merging #5036 (2a8981b) into master (6370189) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           master    #5036    +/-   ##
========================================
  Coverage   98.23%   98.24%            
========================================
  Files         381      383     +2     
  Lines       35873    35974   +101     
========================================
+ Hits        35241    35342   +101     
  Misses        632      632            
Impacted Files Coverage Δ
lib/OpenQA/CLI/api.pm 100.00% <100.00%> (ø)
lib/OpenQA/CLI/schedule.pm 100.00% <100.00%> (ø)
lib/OpenQA/Command.pm 100.00% <100.00%> (ø)
t/43-cli-schedule.t 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

okurz
okurz previously requested changes Mar 15, 2023
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

This looks good!

@grisu48 as the author of openqa-mon, what do you think about this approach? Is it a good complementary solution, something completely different or duplicating behaviour?

lib/OpenQA/CLI/schedule.pm Outdated Show resolved Hide resolved
lib/OpenQA/CLI/schedule.pm Show resolved Hide resolved
lib/OpenQA/CLI/schedule.pm Outdated Show resolved Hide resolved
lib/OpenQA/CLI/schedule.pm Show resolved Hide resolved
lib/OpenQA/Command.pm Outdated Show resolved Hide resolved
Copy link
Member

@kraih kraih left a comment

Choose a reason for hiding this comment

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

Pretty sure this breaks openqa-cli api JSON output if the JSON contains unicode characters.

lib/OpenQA/CLI/schedule.pm Outdated Show resolved Hide resolved
Martchus added a commit to Martchus/openQA that referenced this pull request Mar 20, 2023
As mentioned in os-autoinst#5036 (review)
this otherwise breaks outputting JSON.
@Martchus Martchus requested a review from kraih March 20, 2023 12:57
lib/OpenQA/CLI/schedule.pm Outdated Show resolved Hide resolved
When configuring exactly one retry, the loop currently does the
following (assuming the request fails and should be retried):
```
1. Request
2. Log "Request failed" and sleep for the configured retry delay
```
So no retry is done despite being logged and sleep being
executed. This is identical to zero retries being configured
except for 2. but should actually lead to two attempts in total.
If more than one retry is configured the problem is the same of
course.

This change alters the behavior to be (and by the way needs two
less lines of code):
```
1. Request
2. Log "Request failed" and sleep for the configured retry delay
3. Do actually one more request
```
As mentioned in os-autoinst#5036 (review)
this otherwise breaks outputting JSON.
@grisu48
Copy link
Contributor

grisu48 commented Mar 20, 2023

This looks good!

@grisu48 as the author of openqa-mon, what do you think about this approach? Is it a good complementary solution, something completely different or duplicating behaviour?

No concerns by me. openqa-mon does more than just waiting and having this as complementary option directly in openqa-cli is a nice-to-have.

t/43-cli-schedule.t Outdated Show resolved Hide resolved
* Declare `$res` where is is actually used
* Fix naming of command object
@Martchus Martchus dismissed okurz’s stale review March 20, 2023 16:50

All points have been resolved

@Martchus Martchus removed the request for review from okurz March 20, 2023 16:51
@mergify mergify bot merged commit 529c547 into os-autoinst:master Mar 20, 2023
@Martchus Martchus deleted the schedule-iso branch March 20, 2023 21:22
os-autoinst-bot pushed a commit to os-autoinst-bot/openQA that referenced this pull request Mar 21, 2023
commit 529c547
Merge: 84fc5a7 2a8981b
Author:     mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
AuthorDate: Mon Mar 20 18:52:16 2023 +0000
Commit:     GitHub <noreply@github.com>
CommitDate: Mon Mar 20 18:52:16 2023 +0000

    Merge pull request os-autoinst#5036 from Martchus/schedule-iso

    Allow scheduling and monitoring a product via `openqa-cli`
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.

6 participants