Skip to content

Rewrite openqa-advanced-retrigger-jobs in python#332

Draft
okurz wants to merge 2 commits into
os-autoinst:masterfrom
okurz:feature/openqa_advanced_retrigger_python2
Draft

Rewrite openqa-advanced-retrigger-jobs in python#332
okurz wants to merge 2 commits into
os-autoinst:masterfrom
okurz:feature/openqa_advanced_retrigger_python2

Conversation

@okurz
Copy link
Copy Markdown
Member

@okurz okurz commented Jul 23, 2024

No description provided.

perlpunk

This comment was marked as resolved.

Sign in to view

@okurz

This comment was marked as resolved.

Sign in to view
aylen384
aylen384 reviewed Jul 23, 2024
View reviewed changes
Comment thread openqa-advanced-retrigger-jobs Outdated
@perlpunk

This comment was marked as resolved.

Sign in to view
@perlpunk
Copy link
Copy Markdown
Contributor

perlpunk commented Jul 23, 2024

Also how about moving scripts to bin/ and adding an extension, .py. I don't have a problem with extensions, and it makes tooling like linting easier

@okurz
Copy link
Copy Markdown
Member Author

okurz commented Jul 23, 2024

Also how about moving scripts to bin/ and adding an extension, .py. I don't have a problem with extensions, and it makes tooling like linting easier

An extension would mean that we would need a separate installation tooling and step to install as executable without extension and I would like to avoid that.

@okurz okurz force-pushed the feature/openqa_advanced_retrigger_python2 branch from f32c1d5 to f8d55f9 Compare July 24, 2024 07:00
@okurz
Copy link
Copy Markdown
Member Author

okurz commented Jul 24, 2024

It would still be good to have an example. I remember that it was complicated to get the quoting right

Done

nicksinger
nicksinger reviewed Jul 24, 2024
View reviewed changes
Comment thread openqa-investigate Outdated
perlpunk
perlpunk reviewed Jul 24, 2024
View reviewed changes
Comment thread openqa-investigate Outdated
perlpunk
perlpunk reviewed Jul 24, 2024
View reviewed changes
Comment thread openqa-investigate Outdated
@okurz okurz force-pushed the feature/openqa_advanced_retrigger_python2 branch from f8d55f9 to d00d067 Compare July 24, 2024 10:39
@mergify mergify Bot mentioned this pull request Jul 24, 2024
Draft
@okurz okurz force-pushed the feature/openqa_advanced_retrigger_python2 branch 2 times, most recently from bd97f6e to bdd1863 Compare July 25, 2024 18:12
@okurz
Copy link
Copy Markdown
Member Author

okurz commented Jul 25, 2024

Updated:

  • Simple and advanced example
  • Converted single string in subprocess.run with shell=True into list (or actually a set). For that I extracted the post function so that I can just print in case of "dry-run" instead of the "client prefix" which was previously used to "echo" instead in bash.

Tested manually with

while_inotifywait sh -c "python3.11 ./openqa-advanced-retrigger-jobs -vvvv --host openqa.suse.de --failed-since \"2024-07-23T13:00\" --result failed --additional-filters \"test not like '%:investigate:%' and id=14995768\" --comment \"my foo comment\" --dry-run && python3.11 ./openqa-advanced-retrigger-jobs -vvvv --host openqa.suse.de --failed-since \"2024-07-23T13:00\" --result failed --additional-filters \"test not like '%:investigate:%' and id=14995768\" --comment \"my foo comment\""

@okurz okurz removed the not-ready label Jul 25, 2024
Martchus
Martchus reviewed Jul 29, 2024
View reviewed changes
Comment thread openqa-advanced-retrigger-jobs
Comment on lines +89 to +92
query = (
f"select id from jobs where ({worker_string}result='{args.result}' "
f"and clone_id is null and t_finished >= '{args.failed_since}'{additional_filters});"
)
Copy link
Copy Markdown
Contributor

@Martchus Martchus Jul 29, 2024

Choose a reason for hiding this comment

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

Normally I'd say this needs escaping but of course the previous script also didn't have that…

Copy link
Copy Markdown
Member Author

@okurz okurz Jul 29, 2024

Choose a reason for hiding this comment

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

I don't know what additional escaping you mean. Why do you mean we need escaping?

Copy link
Copy Markdown
Member

@nicksinger nicksinger Jul 29, 2024

Choose a reason for hiding this comment

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

I think he means something in the lines of https://www.psycopg.org/psycopg3/docs/basic/params.html#execute-arguments

Currently e.g. args.result could be used to insert arbitrary SQL statements

Copy link
Copy Markdown
Member

@nicksinger nicksinger Jul 29, 2024

Choose a reason for hiding this comment

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

While malice is very unlikely (given you'd need access anyway) it could help if people accidentally pass arguments which cause the query to break

Copy link
Copy Markdown
Contributor

@Martchus Martchus Jul 29, 2024

Choose a reason for hiding this comment

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

And one can also (accidentally) break out of the whole psql invocation.

Comment thread openqa-advanced-retrigger-jobs Outdated
Comment thread openqa-advanced-retrigger-jobs Outdated
@okurz okurz force-pushed the feature/openqa_advanced_retrigger_python2 branch from 9725eaf to 00ab178 Compare July 29, 2024 11:58
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 4, 2025

This pull request is now in conflicts. Could you fix it? 🙏

@okurz okurz marked this pull request as draft March 19, 2026 06:30
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 19, 2026

This pull request is now in conflicts. Could you fix it? 🙏

@okurz okurz force-pushed the feature/openqa_advanced_retrigger_python2 branch from 00ab178 to d4cb020 Compare May 6, 2026 12:50
@okurz okurz force-pushed the feature/openqa_advanced_retrigger_python2 branch from d4cb020 to 1a72265 Compare May 8, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@perlpunk perlpunk perlpunk left review comments

@aylen384 aylen384 aylen384 left review comments

@Martchus Martchus Martchus left review comments

@nicksinger nicksinger nicksinger left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@okurz @perlpunk @aylen384 @Martchus @nicksinger