consumer, scheduler: support testing dist-git PRs (#104) #143

Merged

adamwill merged 1 commit from schedule-distgit-prs into main 2026-07-11 00:57:12 +00:00

Conversation 11 Commits 1 Files changed 11 +650 -43

adamwill commented 2026-07-03 21:43:34 +00:00

Owner

Copy link

This makes it possible to request openQA tests for dist-git PRs.
Anyone (for now) can comment "/openqa test" on a dist-git PR,
and if the consumer is configured to listen on the appropriate
topics, it should schedule update tests (always the full set, for
now) on the scratch build associated with the PR. If the scratch
build is complete when the comment is added, the tests should be
scheduled immediately; if not, the tests will be scheduled when
we get a message indicating a scratch build has completed for the
PR. We also automatically test any subsequent scratch builds for
the same PR when they complete.

As part of this, we move the openqa_hostname and openqa_baseurl
definitions from consumer config to fedora_openqa config. They
are always the same for all consumers in real-world use, so it
doesn't really make sense to require them to be set for every
consumer.

Signed-off-by: Adam Williamson awilliam@redhat.com

This makes it possible to request openQA tests for dist-git PRs. Anyone (for now) can comment "/openqa test" on a dist-git PR, and if the consumer is configured to listen on the appropriate topics, it should schedule update tests (always the full set, for now) on the scratch build associated with the PR. If the scratch build is complete when the comment is added, the tests should be scheduled immediately; if not, the tests will be scheduled when we get a message indicating a scratch build has completed for the PR. We also automatically test any subsequent scratch builds for the same PR when they complete. As part of this, we move the openqa_hostname and openqa_baseurl definitions from consumer config to fedora_openqa config. They are always the same for all consumers in real-world use, so it doesn't really make sense to require them to be set for every consumer. Signed-off-by: Adam Williamson <awilliam@redhat.com>

adamwill added 1 commit 2026-07-03 21:43:34 +00:00

consumer, scheduler: support testing dist-git PRs (#104) ... ae3aac2bd3

This makes it possible to request openQA tests for dist-git PRs.
Anyone (for now) can comment "/openqa test" on a dist-git PR,
and if the consumer is configured to listen on the appropriate
topics, it should schedule update tests (always the full set, for
now) on the scratch build associated with the PR. If the scratch
build is complete when the comment is added, the tests should be
scheduled immediately; if not, the tests will be scheduled when
we get a message indicating a scratch build has completed for the
PR. We also automatically test any subsequent scratch builds for
the same PR when they complete.

As part of this, we move the openqa_hostname and openqa_baseurl
definitions from consumer config to fedora_openqa config. They
are always the same for all consumers in real-world use, so it
doesn't really make sense to require them to be set for every
consumer.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

adamwill added the

ai-review-please

label 2026-07-03 21:54:39 +00:00

forgejo-actions removed the

ai-review-please

label 2026-07-03 21:54:42 +00:00

forgejo-actions commented 2026-07-03 21:54:57 +00:00

Copy link

AI Code Review

Detailed Code Review

The PR introduces functionality to automatically trigger and report on openQA tests for dist-git pull requests, which is a significant workflow enhancement. Moving the openQA URLs to a centralized configuration is a good refactor. However, there are a few potential issues related to dictionary key handling, robust string matching, and URL construction that should be addressed to ensure stability. Specifically, dictionary accesses like comment["comment"] on potentially varied Pagure payloads should be done carefully to avoid KeyError, and splitting arbitrary URLs or checking for specific branches requires more robust error handling.

📂 File Reviews

📄 `src/fedora_openqa/consumer.py` - Introduces logic for scheduling openQA jobs based on Pagure PR comments and flags, and includes a new reporter for pushing results back to dist-git.

• Major [Bug]: Missing dictionary key check for comment["comment"]. If a comment dictionary doesn't contain the 'comment' key, this will raise a KeyError.
  • Suggestion: Use .get("comment") instead of direct bracket access.

if not comment.get("comment") == "/openqa test":
    self.logger.debug("Not a comment requesting openQA tests")
    return

• Major [Bug]: Same issue with missing dictionary key check for comment["comment"] in _consume_pr_flag list comprehension.
  • Suggestion: Use .get("comment") in the any() generator expression.

if not any(comment.get("comment") == "/openqa test" for comment in pr.get("comments", [])):
    return

• Minor [Logic]: The logic not f" {branch} " in flag["username"] might be brittle if Pagure/Packit changes how they format usernames in scratch build flags.

  • Suggestion: Consider a more robust matching strategy, or at least document this dependency on Packit's flag naming convention.

• Minor [Bug]: _version_from_branch is defined outside a class but accepts self as the first argument, behaving like a method. However, it's called as _version_from_branch(self, body).

  • Suggestion: If it doesn't need to be part of the class, remove self and just pass logger (or self.logger), or move it inside the class as a method.

def _version_from_branch(logger, body):
    # ...
    logger.debug("PR branch %s appears to not be a Fedora mainline release branch", branch)
    # ...

# Call site:
version, branch = _version_from_branch(self.logger, body)

📄 `src/fedora_openqa/schedule.py` - Updates the `jobs_from_update` function to correctly parse task IDs from PR strings.

• Minor [Logic]: update.startswith("PR-") could fail if update is a list, since strings have startswith but lists do not. The check isinstance(update, list) or update.startswith("PR-") works because Python short-circuits, but if update is neither a list nor a string, it will throw an AttributeError.
  • Suggestion: Check if update is a string before calling startswith.

if isinstance(update, list) or (isinstance(update, str) and update.startswith("PR-")):

✅ Summary

• Overall Assessment: The changes successfully implement dist-git testing capabilities, though a few potential bugs with dictionary accesses and function signatures should be fixed to prevent runtime exceptions.
• Priority Issues:
  • Fix KeyError risks by using .get("comment") instead of ["comment"] on Pagure comment objects.
  • Refactor _version_from_branch to either be a proper class method or pass the logger directly, rather than passing self to a standalone function.
  • Ensure update.startswith("PR-") in schedule.py is protected by a string type check to prevent AttributeError.
• Minor Suggestions:
  • Make Packit username string matching more robust if possible.

---

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### Detailed Code Review The PR introduces functionality to automatically trigger and report on openQA tests for dist-git pull requests, which is a significant workflow enhancement. Moving the openQA URLs to a centralized configuration is a good refactor. However, there are a few potential issues related to dictionary key handling, robust string matching, and URL construction that should be addressed to ensure stability. Specifically, dictionary accesses like `comment["comment"]` on potentially varied Pagure payloads should be done carefully to avoid KeyError, and splitting arbitrary URLs or checking for specific branches requires more robust error handling. #### 📂 File Reviews <details> <summary><strong>📄 `src/fedora_openqa/consumer.py`</strong> - Introduces logic for scheduling openQA jobs based on Pagure PR comments and flags, and includes a new reporter for pushing results back to dist-git.</summary> - **Major** [Bug]: Missing dictionary key check for `comment["comment"]`. If a comment dictionary doesn't contain the 'comment' key, this will raise a KeyError. - *Suggestion:* Use `.get("comment")` instead of direct bracket access. ```` if not comment.get("comment") == "/openqa test": self.logger.debug("Not a comment requesting openQA tests") return ```` - **Major** [Bug]: Same issue with missing dictionary key check for `comment["comment"]` in `_consume_pr_flag` list comprehension. - *Suggestion:* Use `.get("comment")` in the `any()` generator expression. ```` if not any(comment.get("comment") == "/openqa test" for comment in pr.get("comments", [])): return ```` - **Minor** [Logic]: The logic `not f" {branch} " in flag["username"]` might be brittle if Pagure/Packit changes how they format usernames in scratch build flags. - *Suggestion:* Consider a more robust matching strategy, or at least document this dependency on Packit's flag naming convention. - **Minor** [Bug]: `_version_from_branch` is defined outside a class but accepts `self` as the first argument, behaving like a method. However, it's called as `_version_from_branch(self, body)`. - *Suggestion:* If it doesn't need to be part of the class, remove `self` and just pass `logger` (or `self.logger`), or move it inside the class as a method. ```` def _version_from_branch(logger, body): # ... logger.debug("PR branch %s appears to not be a Fedora mainline release branch", branch) # ... # Call site: version, branch = _version_from_branch(self.logger, body) ```` </details> <details> <summary><strong>📄 `src/fedora_openqa/schedule.py`</strong> - Updates the `jobs_from_update` function to correctly parse task IDs from PR strings.</summary> - **Minor** [Logic]: `update.startswith("PR-")` could fail if `update` is a list, since strings have `startswith` but lists do not. The check `isinstance(update, list) or update.startswith("PR-")` works because Python short-circuits, but if `update` is neither a list nor a string, it will throw an AttributeError. - *Suggestion:* Check if `update` is a string before calling `startswith`. ```` if isinstance(update, list) or (isinstance(update, str) and update.startswith("PR-")): ```` </details> ### ✅ Summary - **Overall Assessment:** The changes successfully implement dist-git testing capabilities, though a few potential bugs with dictionary accesses and function signatures should be fixed to prevent runtime exceptions. - **Priority Issues:** - Fix `KeyError` risks by using `.get("comment")` instead of `["comment"]` on Pagure comment objects. - Refactor `_version_from_branch` to either be a proper class method or pass the logger directly, rather than passing `self` to a standalone function. - Ensure `update.startswith("PR-")` in `schedule.py` is protected by a string type check to prevent `AttributeError`. - **Minor Suggestions:** - Make Packit username string matching more robust if possible. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*

adamwill force-pushed schedule-distgit-prs from ae3aac2bd3 to a488ba9912 2026-07-03 21:57:38 +00:00 Compare

adamwill force-pushed schedule-distgit-prs from a488ba9912 to 869b8518d8 2026-07-03 22:02:38 +00:00 Compare

adamwill commented 2026-07-03 22:10:27 +00:00

Author

Owner

Copy link

Fixed some of the AI review stuff.

Fixed some of the AI review stuff.

adamwill added the

ai-review-please

label 2026-07-03 22:10:36 +00:00

forgejo-actions removed the

ai-review-please

label 2026-07-03 22:10:38 +00:00

forgejo-actions commented 2026-07-03 22:10:50 +00:00

Copy link

AI Code Review

Detailed Code Review

The PR introduces functionality to trigger openQA tests for dist-git PRs using a comment command (/openqa test). The implementation correctly relocates openqa_hostname and openqa_baseurl to the global fedora_openqa config. The logic correctly handles both immediate triggering and queuing for asynchronous build completions. However, the security model around who can trigger tests remains completely open, posing a denial of service or resource exhaustion risk. There are also potential edge cases with string manipulations, API responses, and exception handling.

✅ Summary

• Overall Assessment: The changes generally work as intended based on the logic provided, but an open authorization model for test triggering presents a security/resource risk that should be addressed before deployment.
• Priority Issues:
  • Unrestricted access to the /openqa test command could lead to resource exhaustion if malicious or accidental users trigger builds continuously.
  • No retry logic or robust error handling for external requests (like setting PR flags via requests.post), which can fail silently except for a log message.
  • String parsing for task IDs assumes precise formats without sufficient validation, which could throw IndexError.
• Minor Suggestions:
  • Use URL query building tools instead of manual string concatenation for overviewurl.
  • Centralize the parsing of BUILD strings into a helper function to avoid repeated brittle split operations.
  • Implement a timeout on external API requests using the requests library.

---

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### Detailed Code Review The PR introduces functionality to trigger openQA tests for dist-git PRs using a comment command (`/openqa test`). The implementation correctly relocates `openqa_hostname` and `openqa_baseurl` to the global `fedora_openqa` config. The logic correctly handles both immediate triggering and queuing for asynchronous build completions. However, the security model around who can trigger tests remains completely open, posing a denial of service or resource exhaustion risk. There are also potential edge cases with string manipulations, API responses, and exception handling. ### ✅ Summary - **Overall Assessment:** The changes generally work as intended based on the logic provided, but an open authorization model for test triggering presents a security/resource risk that should be addressed before deployment. - **Priority Issues:** - Unrestricted access to the `/openqa test` command could lead to resource exhaustion if malicious or accidental users trigger builds continuously. - No retry logic or robust error handling for external requests (like setting PR flags via `requests.post`), which can fail silently except for a log message. - String parsing for task IDs assumes precise formats without sufficient validation, which could throw `IndexError`. - **Minor Suggestions:** - Use URL query building tools instead of manual string concatenation for `overviewurl`. - Centralize the parsing of `BUILD` strings into a helper function to avoid repeated brittle `split` operations. - Implement a timeout on external API requests using the `requests` library. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*

adamwill force-pushed schedule-distgit-prs from 869b8518d8 to f5aac5ca5e 2026-07-03 22:48:49 +00:00 Compare

adamwill commented 2026-07-03 22:49:56 +00:00

Author

Owner

Copy link

Added retries and timeout to the requests stuff, and used urlencode for overviewurl.

Added retries and timeout to the requests stuff, and used urlencode for overviewurl.

adamwill added the

ai-review-please

label 2026-07-03 22:50:02 +00:00

forgejo-actions removed the

ai-review-please

label 2026-07-03 22:50:06 +00:00

forgejo-actions commented 2026-07-03 22:50:19 +00:00

Copy link

AI Code Review

Detailed Code Review

The author implemented the logic to schedule openQA tests for dist-git PRs based on incoming fedmsg (Pagure) comments and flag updates, migrating configuration parameters for shared usage. Retries and basic URL encoding were applied. The logic accurately monitors '/openqa test' PR comments, retrieves scratch build tasks, schedules the appropriate openQA tests, and reports success, failure, or error back to the dist-git PR flag. Overall, the approach is architecturally sound and integrates neatly into the established consumer frameworks.

However, a logic issue exists when fetching properties from nested dictionaries where defaults are expected to be an empty string but are missing .get fallback in secondary indexing (e.g., prid construction). There is also a missing input validation for branch format matching in the mainline checker, but since it is handled softly it mostly just returns an empty string. The PR ID parsing splits on '-' with maxsplit 2, which could fail if the repository string itself has extra hyphens in unexpected structures.

✅ Summary

• Overall Assessment: The changes successfully add dist-git PR support with centralized openQA configuration. A few minor edge cases with data parsing could cause exceptions on unexpected payload structures.
• Minor Suggestions:
  • Safeguard prid construction in _consume_pr_comment and _consume_pr_flag against pr.get("project", {}) being None, as get() defaults to {} but might return None if the key exists with a null value.
  • Handle potential ValueError in build.replace("-NOREPORT", "").split("-", 2) if the BUILD string unexpectedly does not contain exactly two hyphens after substitution.

---

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### Detailed Code Review The author implemented the logic to schedule openQA tests for dist-git PRs based on incoming fedmsg (Pagure) comments and flag updates, migrating configuration parameters for shared usage. Retries and basic URL encoding were applied. The logic accurately monitors '/openqa test' PR comments, retrieves scratch build tasks, schedules the appropriate openQA tests, and reports success, failure, or error back to the dist-git PR flag. Overall, the approach is architecturally sound and integrates neatly into the established consumer frameworks. However, a logic issue exists when fetching properties from nested dictionaries where defaults are expected to be an empty string but are missing `.get` fallback in secondary indexing (e.g., `prid` construction). There is also a missing input validation for branch format matching in the mainline checker, but since it is handled softly it mostly just returns an empty string. The PR ID parsing splits on '-' with maxsplit 2, which could fail if the repository string itself has extra hyphens in unexpected structures. ### ✅ Summary - **Overall Assessment:** The changes successfully add dist-git PR support with centralized openQA configuration. A few minor edge cases with data parsing could cause exceptions on unexpected payload structures. - **Minor Suggestions:** - Safeguard `prid` construction in `_consume_pr_comment` and `_consume_pr_flag` against `pr.get("project", {})` being None, as `get()` defaults to `{}` but might return `None` if the key exists with a null value. - Handle potential `ValueError` in `build.replace("-NOREPORT", "").split("-", 2)` if the `BUILD` string unexpectedly does not contain exactly two hyphens after substitution. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*

adamwill commented 2026-07-03 22:54:10 +00:00

Author

Owner

Copy link

I love the smell of "exceptions on unexpected payload structures" in the morning. Makes me feel alive.

I love the smell of "exceptions on unexpected payload structures" in the morning. Makes me feel alive.

adamwill commented 2026-07-03 22:55:54 +00:00

Author

Owner

Copy link

@lruzicka wdyt?

@lruzicka wdyt?

adamwill force-pushed schedule-distgit-prs from f5aac5ca5e to 177e6093e4 2026-07-04 00:02:07 +00:00 Compare

adamwill commented 2026-07-04 00:02:19 +00:00

Author

Owner

Copy link

cleaned up the tests a bit with some of my beloved parametrization.

cleaned up the tests a bit with some of my beloved parametrization.

adamwill added 1 commit 2026-07-08 00:14:33 +00:00

schedule only appropriate flavors based on critpath groups ... c1ac713c25

Signed-off-by: Adam Williamson <awilliam@redhat.com>

adamwill force-pushed schedule-distgit-prs from c1ac713c25 to 06dd07fe02 2026-07-08 00:18:47 +00:00 Compare

adamwill force-pushed schedule-distgit-prs from 06dd07fe02 to 76c9942bef 2026-07-08 00:43:54 +00:00 Compare

adamwill force-pushed schedule-distgit-prs from 76c9942bef to ee4a4f5f18 2026-07-08 18:34:42 +00:00 Compare

adamwill force-pushed schedule-distgit-prs from ee4a4f5f18 to 09be49a7fa 2026-07-08 18:37:27 +00:00 Compare

adamwill commented 2026-07-09 01:52:20 +00:00

Author

Owner

Copy link

I've done some fixes to this for current Rawhide, and to fix the test on GNOME (there was a needle tag mismatch). More tests running now.

I've done some fixes to this for current Rawhide, and to fix the test on GNOME (there was a needle tag mismatch). More tests running now.

adamwill force-pushed schedule-distgit-prs from 09be49a7fa to e7df0f5214 2026-07-11 00:38:21 +00:00 Compare

adamwill added the

ai-review-please

label 2026-07-11 00:51:20 +00:00

forgejo-actions removed the

ai-review-please

label 2026-07-11 00:51:24 +00:00

forgejo-actions commented 2026-07-11 00:52:02 +00:00

Copy link

AI Code Review

Detailed Code Review

The PR introduces functional additions to trigger openQA testing for dist-git PRs via message bus consumption. The author has proactively addressed several stability concerns, such as configuring a requests session with robust retry logic and correctly isolating configuration. However, the implementation is currently vulnerable to message processing hangs due to a missing HTTP timeout on a secondary API call, and there are a couple of fragile data extraction patterns from the Pagure message payload that could lead to unhandled exceptions (and thus consumer crashes) under edge cases.

📂 File Reviews

📄 `src/fedora_openqa/consumer.py` - Introduces logic to consume PR comments and flag updates, scheduling openQA tests for dist-git PRs.

• Critical [Bug]: The SESSION.get() call to Bodhi for critpath components is missing a timeout. If the Bodhi service hangs or becomes unresponsive, the fedora-messaging consumer will block indefinitely, preventing the consumption of any further messages.
  • Suggestion: Add an explicit timeout to the request, consistent with the other SESSION requests in this file.

critpath = SESSION.get("https://bodhi.fedoraproject.org/get_critpath_components", timeout=60).json()

• Major [Bug]: Extracting the last comment using pr.get("comments", [{}])[-1] is vulnerable to an IndexError. If the Pagure API payload includes an explicitly empty list ("comments": []), pr.get() will return [], and [-1] will raise an IndexError, causing the consumer to crash for that message.
  • Suggestion: Safely extract the comments list and verify it is not empty before accessing the last element.

comments = pr.get("comments") or []
if not comments:
    return
comment = comments[-1]

• Minor [Logic]: In _consume_pr_flag, the fallback for pr.get("comments") is an empty dictionary ({}). While iterating over an empty dictionary is safe, if the field ever contained a populated dictionary, iterating over it would yield string keys, leading to an AttributeError when comment.get("comment", "") is called. It is safer and more semantically correct to default to an empty list.
  • Suggestion: Change the default fallback from {} to [].

if not any(comment.get("comment", "") == "/openqa test" for comment in pr.get("comments", [])):

• Minor [Bug]: In _jobs_from_flag, if the flag payload explicitly sets the username to None, flag.get("username", "") will return None (not ""). The subsequent in operator string check will raise a TypeError.
  • Suggestion: Ensure the username resolves to a string before using the in operator.

if not f" {branch} " in (flag.get("username") or ""):

✅ Summary

• Overall Assessment: The core logic and messaging interactions are well-structured, but critical missing network timeouts and potential unhandled exceptions during JSON payload parsing must be fixed to ensure the message consumer remains robust and stable.
• Priority Issues:
  • Missing timeout on the Bodhi critpath API request, risking message queue blockages.
  • Potential IndexError when extracting PR comments if the payload explicitly provides an empty list.
  • Potential TypeError on flag.get('username') if the API explicitly returns None.
• Minor Suggestions:
  • Use empty list [] instead of empty dict {} as the default fallback when getting the 'comments' array.

---

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### Detailed Code Review The PR introduces functional additions to trigger openQA testing for dist-git PRs via message bus consumption. The author has proactively addressed several stability concerns, such as configuring a requests session with robust retry logic and correctly isolating configuration. However, the implementation is currently vulnerable to message processing hangs due to a missing HTTP timeout on a secondary API call, and there are a couple of fragile data extraction patterns from the Pagure message payload that could lead to unhandled exceptions (and thus consumer crashes) under edge cases. #### 📂 File Reviews <details> <summary><strong>📄 `src/fedora_openqa/consumer.py`</strong> - Introduces logic to consume PR comments and flag updates, scheduling openQA tests for dist-git PRs.</summary> - **Critical** [Bug]: The `SESSION.get()` call to Bodhi for critpath components is missing a `timeout`. If the Bodhi service hangs or becomes unresponsive, the fedora-messaging consumer will block indefinitely, preventing the consumption of any further messages. - *Suggestion:* Add an explicit timeout to the request, consistent with the other `SESSION` requests in this file. ```` critpath = SESSION.get("https://bodhi.fedoraproject.org/get_critpath_components", timeout=60).json() ```` - **Major** [Bug]: Extracting the last comment using `pr.get("comments", [{}])[-1]` is vulnerable to an `IndexError`. If the Pagure API payload includes an explicitly empty list (`"comments": []`), `pr.get()` will return `[]`, and `[-1]` will raise an `IndexError`, causing the consumer to crash for that message. - *Suggestion:* Safely extract the comments list and verify it is not empty before accessing the last element. ```` comments = pr.get("comments") or [] if not comments: return comment = comments[-1] ```` - **Minor** [Logic]: In `_consume_pr_flag`, the fallback for `pr.get("comments")` is an empty dictionary (`{}`). While iterating over an empty dictionary is safe, if the field ever contained a populated dictionary, iterating over it would yield string keys, leading to an `AttributeError` when `comment.get("comment", "")` is called. It is safer and more semantically correct to default to an empty list. - *Suggestion:* Change the default fallback from `{}` to `[]`. ```` if not any(comment.get("comment", "") == "/openqa test" for comment in pr.get("comments", [])): ```` - **Minor** [Bug]: In `_jobs_from_flag`, if the flag payload explicitly sets the `username` to `None`, `flag.get("username", "")` will return `None` (not `""`). The subsequent `in` operator string check will raise a `TypeError`. - *Suggestion:* Ensure the username resolves to a string before using the `in` operator. ```` if not f" {branch} " in (flag.get("username") or ""): ```` </details> ### ✅ Summary - **Overall Assessment:** The core logic and messaging interactions are well-structured, but critical missing network timeouts and potential unhandled exceptions during JSON payload parsing must be fixed to ensure the message consumer remains robust and stable. - **Priority Issues:** - Missing timeout on the Bodhi critpath API request, risking message queue blockages. - Potential IndexError when extracting PR comments if the payload explicitly provides an empty list. - Potential TypeError on flag.get('username') if the API explicitly returns None. - **Minor Suggestions:** - Use empty list [] instead of empty dict {} as the default fallback when getting the 'comments' array. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*

adamwill force-pushed schedule-distgit-prs from e7df0f5214 to ac5a56f41b 2026-07-11 00:56:41 +00:00 Compare

adamwill commented 2026-07-11 00:56:55 +00:00

Author

Owner

Copy link

Timeout added. The other things aren't worth worrying about in context.

Timeout added. The other things aren't worth worrying about in context.

adamwill merged commit ac5a56f41b into main 2026-07-11 00:57:12 +00:00

adamwill deleted branch schedule-distgit-prs 2026-07-11 00:57:12 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

Closed As

Duplicate

Closed with the reason of Duplicate

  

Closed As

Fixed

Closed with the reason of Fixed

  

Closed As

Invalid

Closed with the reason of Invalid

  

easyfix

  

enhancement

  

ai-review-please

Request AI review for a pull request

  

Backlog Status

Needs Review

Work items that have been created or updated but need review, refinement, or approval before they can be considered ready for sprint planning.

  

Backlog Status

Ready

Work items that have been reviewed, refined, and are ready to be committed to a sprint. These items have clear requirements and can be worked on immediately when capacity is available.

  

chore

Work that needs to be done but doesn't add new functionality or fix bugs. Maintenance tasks that keep the system running smoothly and the codebase clean.

  

documentation

Work focused on creating, updating, or improving documentation for users, developers, or system administrators.

  

points

01

A task that is simple, small, and has very little uncertainty. It should be easy enough for any team member to pick up and complete quickly, without a lot of discussion or research.

  

points

02

This is a task that is slightly more complex or larger than a "1." It might involve two or three simple steps, but the outcome is still very clear and there's little uncertainty.

  

points

03

Medium-sized task with some noticeable complexity. It might require a bit more thought or collaboration. The steps are well-defined, but it's not a trivial change.

  

points

05

A significant piece of work. It has more complexity and a higher degree of uncertainty than smaller tasks. It might require some research, a design discussion, or collaboration across multiple teams.

  

points

08

Complex task with a lot of uncertainty. It's often the largest size that a team will consider for a single sprint. It likely requires a detailed breakdown and multiple team members to complete.

  

points

13

Any task that feels like it's a "13" or more is considered an Epic. These are too large and uncertain to be worked on in a single sprint and should be broken down.

  

Priority

Critical

This is for work that is urgent and of critical importance and so and must be addressed immediately with a higher priority than all other work, even if not originally planned for the current sprint.

  

Priority

High

This is for work that is critical and needs to be addressed in the next sprint. It's often for urgent bugs, important features, or a major piece of technical debt that is blocking other work.

  

Priority

Low

This is for work that is good to do but not urgent. It's a candidate for being worked on when there's extra time or when it rises in importance. It can stay in the backlog for a longer period.

  

Priority

Medium

This is for important work that should be done soon, but isn't as urgent as a high-priority item. It will likely be considered for the next 1-2 sprints.

  

Sprint Status

Blocked

Work items that cannot progress due to external dependencies, blockers, or issues that need resolution.

  

Sprint Status

Done

Work items that have been completed, reviewed, and integrated. No further work is needed within the current sprint.

  

Sprint Status

In Progress

Work items currently being worked on by a team member. Active development, research, or implementation is happening.

  

Sprint Status

Review

Work items that are complete but need review, testing, or approval before being marked as done.

  

Sprint Status

To Do

Work items that are committed to the sprint but haven't been started yet. These items are ready to be picked up by team members.

  

Technical Debt

Work that addresses shortcuts, workarounds, or suboptimal implementations that were done previously to meet deadlines. These items improve code quality, maintainability, or performance without adding new features.

  

Work Item

Bug

Issues where the system is not working as intended or designed. Bugs represent deviations from expected behavior.

  

Work Item

Epic

Large initiatives that are broken down into multiple smaller work items (user stories, tasks, bugs). Epics represent significant features or projects that span multiple sprints.

  

Work Item

Spike

Time-boxed research, investigation, or proof-of-concept work to reduce uncertainty or explore solutions. Spikes help inform future development decisions.

  

Work Item

Task

Used to break down larger user stories into smaller, manageable implementation pieces. They focus on the specific technical steps needed to deliver the user story's value.

  

Work Item

User Story

User-facing features or functionality that can be framed from a user perspective, describing what someone wants to accomplish and why.

No labels

Closed As

Duplicate

Closed As

Fixed

Closed As

Invalid

easyfix

enhancement

ai-review-please

Backlog Status

Needs Review

Backlog Status

Ready

chore

documentation

points

01

points

02

points

03

points

05

points

08

points

13

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Sprint Status

Blocked

Sprint Status

Done

Sprint Status

In Progress

Sprint Status

Review

Sprint Status

To Do

Technical Debt

Work Item

Bug

Work Item

Epic

Work Item

Spike

Work Item

Task

Work Item

User Story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

quality/fedora_openqa!143

No description provided.