fix(openqa-list-incidents): replace asserts with error handling#580
Open
okurz wants to merge 1 commit into
Open
fix(openqa-list-incidents): replace asserts with error handling#580okurz wants to merge 1 commit into
okurz wants to merge 1 commit into
Conversation
perlpunk
approved these changes
May 11, 2026
d3flex
reviewed
May 12, 2026
| job = pathlib.Path(url.path).name.removeprefix("t") | ||
|
|
||
| assert job.isdigit() | ||
| if not job.isdigit(): |
Contributor
There was a problem hiding this comment.
that comes from the openqa API resp, if that ever happens means that we do something wrong in openQA
d3flex
reviewed
May 12, 2026
| except RequestException as error: | ||
| sys.exit(f"ERROR: {job}: {error}") | ||
| assert len(data) == 1 | ||
| if len(data) != 1: |
Contributor
There was a problem hiding this comment.
this also looks wrong -
I tried
❯ ./openqa-list-incidents "https://openqa.opensuse.org/tests/overview?distri=aeon&distri=microos&distri=opensuse&version=Tumbleweed&build=20260511&groupid=1"
ERROR: https://openqa.opensuse.org/tests/overview?distri=aeon&distri=microos&distri=opensuse&version=Tumbleweed&build=20260511&groupid=1: expected 1 job, got 488
d3flex
requested changes
May 12, 2026
Contributor
d3flex
left a comment
d3flex
left a comment
There was a problem hiding this comment.
I suggest to remove both asserts. there is not serve any really value as is. or the validations should be different
Martchus
approved these changes
May 21, 2026
Motivation: Using 'assert' for runtime validation is discouraged as they can be optimized away and provide poor error messages to the user. Design Choices: - Replaced 'assert' calls in 'get_api_url' with explicit checks that call 'sys.exit' with descriptive error messages. Benefits: More robust error handling and improved user feedback when unexpected API results occur.
okurz
force-pushed
the
fix/style_list_incidents
branch
from
f7be600 to
1d0d776
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Using 'assert' for runtime validation is discouraged as they can be optimized
away and provide poor error messages to the user.
Design Choices:
'sys.exit' with descriptive error messages.
Benefits:
More robust error handling and improved user feedback when unexpected API
results occur.