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 to clone and monitor a job when creating a PR #18697

Merged
merged 3 commits into from Feb 27, 2024

Conversation

Martchus
Copy link
Contributor

@Martchus Martchus commented Feb 20, 2024

This change allows to clone a job when creating a PR by mentioning it in the PR description via @openqa: Clone https://openqa.opensuse.org/….

This requires the split-out monitor-subcommand provided by openQA PR os-autoinst/openQA#5482.

I tested this locally, e.g.:

cat .github/workflows/openqa.yml | yq -r '.jobs.clone_mentioned_job.steps[0].run' | env GH_REPO=foo GH_REF=bar OPENQA_HOST=http://127.0.0.1:9526 OPENQA_API_KEY=… OPENQA_API_SECRET=… GH_PR_BODY="Merge my changes\n\n @openqa: Clone http://127.0.0.1:9526/tests/4240  \nfootnote" DRY_RUN_JOB_IDS='{"4240" : 4246, "4239" : 4245}' bash
Cloned 'http://127.0.0.1:9526/tests/4240' into:
http://127.0.0.1:9526/tests/4246
http://127.0.0.1:9526/tests/4245
script/openqa-cli monitor … 4246 4245
{"blocked_by_id":null,"id":4246,"result":"none","state":"scheduled"}
Job state of job ID 4246: scheduled, waiting …
^C

It also works without DRY_RUN_JOB_IDS; then it actually clones the job and parses the output of openqa-clone-job.


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

Copy link
Contributor

@b10n1k b10n1k left a comment

Choose a reason for hiding this comment

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

Looks good but i want to mention that as it is, it seems to be designed multiple jobs, but it prevents to run simultaneously in both o3 and osd. i guess this is intended.

Another question: o3 and osd should have different keys. How the script knows which to use here?

.github/workflows/openqa.yml Outdated Show resolved Hide resolved
perlpunk

This comment was marked as resolved.

@Martchus
Copy link
Contributor Author

it prevents to run simultaneously in both o3 and osd. i guess this is intended.

Yes, OSD is out of scope for now. I know that limits the usefulness in practice greatly but GitHub just cannot access OSD.

@foursixnine

This comment was marked as resolved.

@perlpunk

This comment was marked as resolved.

@Martchus
Copy link
Contributor Author

I updated this now to use os-autoinst/scripts@29f98b6. I tested it on Martchus#2.

@foursixnine
Copy link
Member

@Martchus keep in mind that @openqa points to a different organization, also can you add some description of how it goes? either to the PR template and to the Contributing file

okurz

This comment was marked as resolved.

@Martchus
Copy link
Contributor Author

Martchus commented Feb 26, 2024

@Martchus keep in mind that @openqa points to a different organization

That's a very good point. We should maybe avoid using the @-syntax here at all because we don't want to mention any entity on GitHub here. I guess the syntax was inspired by Mergify when we created the ticket. However, unlike Mergify this is not implemented as GitHub application so probably it shouldn't pretend to be one. This is something to be fixed in the scripts repository, though.

I create another PR in the scripts repository and with documentation when we figured out what syntax we'd like to use here.

Maybe we can just leave out the @ and make the : mandatory instead.

@okurz
Copy link
Member

okurz commented Feb 26, 2024

@Martchus keep in mind that @openqa points to a different organization, also can you add some description of how it goes? either to the PR template and to the Contributing file

Covered in

This change allows to clone a job when creating a PR by mentioning it in
the PR description via `@openqa: Clone https://openqa.opensuse.org/…`.

Related ticket: https://progress.opensuse.org/issues/130934
CONTRIBUTING.md Outdated Show resolved Hide resolved
foursixnine

This comment was marked as resolved.

Co-authored-by: Santiago Zarate <santiago@zarate.co>
@Martchus Martchus merged commit 08b9325 into os-autoinst:master Feb 27, 2024
9 checks passed
@Martchus Martchus deleted the clone-mentioned-job branch February 27, 2024 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants