Skip to content

Do not fail on release bumping when there are no candidate builds - #761

Open
nforro wants to merge 1 commit into
packit:mainfrom
nforro:release
Open

Do not fail on release bumping when there are no candidate builds#761
nforro wants to merge 1 commit into
packit:mainfrom
nforro:release

Conversation

@nforro

@nforro nforro commented Aug 18, 2026

Copy link
Copy Markdown
Member

@qodo-for-packit

Copy link
Copy Markdown

PR Summary by Qodo

Handle missing candidate builds during release bumping without failing

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Introduce a dedicated exception for “no candidate builds found” lookups.
• Make Z-stream release bumping fall back to spec release when no builds exist.
• Add unit tests covering fallback and failure-vs-empty lookup behavior.
Diagram

graph TD
UT(["UpdateReleaseTool"]) --> U[["ymir.common.utils"]] --> K{{"Koji/Brewhub tag query"}} --> D{"Build exists?"}
D -->|"NoBuildFoundError"| F[/"Spec Release"/] --> R(["Compute new Release"]) --> W[/"Write specfile"/]
D -->|"EVR found"| R
subgraph Legend
  direction LR
  _tool(["Tool"]) ~~~ _mod[["Module"]] ~~~ _ext{{"External"}} ~~~ _file[/"File"/] ~~~ _dec{"Decision"}
end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Return Optional instead of raising for “no builds”
  • ➕ Avoids exception-driven control flow for the expected empty-data case
  • ➕ Makes callers explicitly handle None base-build results
  • ➖ Requires changing the established semantics of get_latest_candidate_build callers
  • ➖ More invasive refactor across the codebase than a typed exception
2. Add an error-code/result object (e.g., Ok/Empty/Error)
  • ➕ Clearly separates empty results from hard failures without exceptions
  • ➕ Scales if more lookup outcomes are needed later
  • ➖ Heavier abstraction for a narrow use case
  • ➖ Requires more widespread API changes and call-site updates

Recommendation: The chosen approach (introducing NoBuildFoundError and only catching it where fallback is valid) is the best tradeoff: it preserves failure behavior for real Koji issues while enabling safe fallback when there are simply no candidate builds yet, with limited API disruption.

Files changed (4) +378 / -64

Bug fix (2) +143 / -62
utils.pyAdd NoBuildFoundError and raise it when tags are empty +5/-1

Add NoBuildFoundError and raise it when tags are empty

• Introduces NoBuildFoundError to represent the “no builds exist in queried tags” condition. Replaces the previous RuntimeError with NoBuildFoundError in the shared latest-build lookup helper, enabling callers to distinguish empty results from operational failures.

ymir/common/utils.py

specfile.pyFallback release bumping when candidate builds are missing +138/-61

Fallback release bumping when candidate builds are missing

• Refactors Release/%dist splitting into a helper and adds logic to resolve the Z-stream base build while tolerating NoBuildFoundError. When no candidate build exists for the current stream, the tool now derives base release from the spec’s current numeric prefix (or 0) and avoids failing; higher-stream absence also falls back to current-stream base when applicable.

ymir/tools/unprivileged/specfile.py

Tests (2) +235 / -2
test_utils.pyAssert typed “no builds” exception for build lookup helpers +3/-2

Assert typed “no builds” exception for build lookup helpers

• Updates unit tests for get_latest_candidate_build and get_latest_z_pending_build to expect NoBuildFoundError instead of a generic RuntimeError when tags contain no builds.

ymir/common/tests/unit/test_utils.py

test_specfile.pyAdd coverage for no-candidate-build Z-stream release scenarios +232/-0

Add coverage for no-candidate-build Z-stream release scenarios

• Adds multiple async unit tests ensuring UpdateReleaseTool succeeds when candidate builds are absent (maintenance and non-maintenance flows), uses specfile fallback correctly, defaults Z-stream counter to 0 when needed, and still fails on real lookup errors (e.g., Koji unavailable).

ymir/tools/unprivileged/tests/unit/test_specfile.py

@qodo-for-packit

qodo-for-packit Bot commented Aug 18, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Unawaited task cancellation ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New
Description
UpdateReleaseTool._resolve_zstream_base_build cancels pending lookup tasks and raises immediately
without awaiting task completion, so the underlying Koji lookups (run via asyncio.to_thread) can
continue consuming executor resources after the tool has failed. If the coroutine is cancelled while
awaiting asyncio.wait, neither lookup task is cancelled/awaited, leaving background work running
beyond the tool invocation.
Code

ymir/tools/unprivileged/specfile.py[R379-382]

+        if fatal_error is not None:
+            for task in pending:
+                task.cancel()
+            raise fatal_error
Relevance

●●● Strong

PR #675 accepted the same asyncio.wait task-cleanup leak pattern, including outer cancellation and
background work concerns.

PR-#675

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The resolver creates tasks and waits with asyncio.wait; on fatal error it calls task.cancel() then
raises without awaiting. Since get_latest_candidate_build is implemented via asyncio.to_thread, the
cancelled asyncio task does not stop the underlying executor work, making proper task cleanup
important to prevent background activity after failure/cancellation.

ymir/tools/unprivileged/specfile.py[367-385]
ymir/common/utils.py[209-215]
PR-#675

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`_resolve_zstream_base_build()` starts two asyncio Tasks and uses `asyncio.wait()`. On fatal error it cancels pending tasks but immediately raises without awaiting their cancellation/termination; and if the parent coroutine is cancelled while awaiting `asyncio.wait()`, the lookup tasks are left running.

This is especially problematic because the lookups ultimately call `asyncio.to_thread(...)`, so cancellation does not stop the underlying blocking Koji call; the best we can do is ensure the asyncio Tasks are cleaned up deterministically and don’t outlive the tool call.

### Issue Context
- The new code uses `asyncio.ensure_future(get_latest_candidate_build(...))` and `asyncio.wait(..., FIRST_EXCEPTION)`.
- `get_latest_candidate_build()` uses `asyncio.to_thread()` internally.

### Fix Focus Areas
- ymir/tools/unprivileged/specfile.py[367-389]

### Suggested change
- Wrap the `asyncio.wait(...)` section in `try/except asyncio.CancelledError` (or `try/finally`) to cancel **both** tasks on parent cancellation.
- When cancelling pending tasks due to `fatal_error`, also `await asyncio.gather(*pending, return_exceptions=True)` (or gather both tasks) before raising, to ensure the loop doesn’t retain pending tasks.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Delayed fatal lookup failure ✓ Resolved 🐞 Bug ☼ Reliability
Description
UpdateReleaseTool._resolve_zstream_base_build uses asyncio.gather(..., return_exceptions=True) for
current/higher stream lookups, which forces waiting for both lookups even when one fails with a real
error. This can significantly delay failure (and potentially hit the tool timeout) if the other Koji
query is slow/blocking, because the underlying Koji calls are synchronous and have no explicit
timeout in this code.
Code

ymir/tools/unprivileged/specfile.py[R366-369]

+            get_latest_candidate_build(package, current_stream_branch),
+            get_latest_candidate_build(package, higher_stream_branch),
+            return_exceptions=True,
+        )
Relevance

●● Moderate

Timeout concern is plausible, but closest precedent is undetermined and timeout additions were
rejected elsewhere.

PR-#654
PR-#526

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR adds a gather(..., return_exceptions=True) path when higher_stream_branch is present,
which guarantees waiting for both build lookups to finish before raising non-NoBuildFoundError
exceptions. The lookups call into Koji via listTagged() without any explicit timeout in this code,
and UpdateReleaseTool has a 30s timeout, making prolonged waits user-visible and potentially causing
tool timeouts.

ymir/tools/unprivileged/specfile.py[365-379]
ymir/tools/unprivileged/specfile.py[198-206]
ymir/common/utils.py[188-197]
ymir/common/utils.py[209-215]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`_resolve_zstream_base_build()` uses `asyncio.gather(..., return_exceptions=True)` to allow treating `NoBuildFoundError` as a non-fatal “no data” result. However, `return_exceptions=True` also makes the function wait for *both* lookups to finish before surfacing any fatal exception, which can delay failures and increase timeout risk when one Koji call is slow.

### Issue Context
- We still need to run both lookups concurrently.
- We must *not* fail when a lookup raises `NoBuildFoundError`.
- We *should* fail fast when a lookup raises anything else (e.g., Koji/network issues), without waiting for the other lookup to complete.

### Fix Focus Areas
- ymir/tools/unprivileged/specfile.py[365-390]

### Suggested implementation direction
- Create tasks for the current/higher lookups.
- Use `asyncio.wait(..., return_when=asyncio.FIRST_EXCEPTION)` (or similar) to detect a fatal exception early.
- If a completed task has an exception that is **not** `NoBuildFoundError`, cancel the pending task(s) and re-raise immediately.
- Otherwise, await the remaining task(s) and then apply the existing selection logic.
- Ensure cancellations are awaited/handled so you don’t leak background work.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 7 rules

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit a9745da

Results up to commit 9add39b ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. Delayed fatal lookup failure ✓ Resolved 🐞 Bug ☼ Reliability
Description
UpdateReleaseTool._resolve_zstream_base_build uses asyncio.gather(..., return_exceptions=True) for
current/higher stream lookups, which forces waiting for both lookups even when one fails with a real
error. This can significantly delay failure (and potentially hit the tool timeout) if the other Koji
query is slow/blocking, because the underlying Koji calls are synchronous and have no explicit
timeout in this code.
Code

ymir/tools/unprivileged/specfile.py[R366-369]

+            get_latest_candidate_build(package, current_stream_branch),
+            get_latest_candidate_build(package, higher_stream_branch),
+            return_exceptions=True,
+        )
Relevance

●● Moderate

Timeout concern is plausible, but closest precedent is undetermined and timeout additions were
rejected elsewhere.

PR-#654
PR-#526

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR adds a gather(..., return_exceptions=True) path when higher_stream_branch is present,
which guarantees waiting for both build lookups to finish before raising non-NoBuildFoundError
exceptions. The lookups call into Koji via listTagged() without any explicit timeout in this code,
and UpdateReleaseTool has a 30s timeout, making prolonged waits user-visible and potentially causing
tool timeouts.

ymir/tools/unprivileged/specfile.py[365-379]
ymir/tools/unprivileged/specfile.py[198-206]
ymir/common/utils.py[188-197]
ymir/common/utils.py[209-215]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`_resolve_zstream_base_build()` uses `asyncio.gather(..., return_exceptions=True)` to allow treating `NoBuildFoundError` as a non-fatal “no data” result. However, `return_exceptions=True` also makes the function wait for *both* lookups to finish before surfacing any fatal exception, which can delay failures and increase timeout risk when one Koji call is slow.

### Issue Context
- We still need to run both lookups concurrently.
- We must *not* fail when a lookup raises `NoBuildFoundError`.
- We *should* fail fast when a lookup raises anything else (e.g., Koji/network issues), without waiting for the other lookup to complete.

### Fix Focus Areas
- ymir/tools/unprivileged/specfile.py[365-390]

### Suggested implementation direction
- Create tasks for the current/higher lookups.
- Use `asyncio.wait(..., return_when=asyncio.FIRST_EXCEPTION)` (or similar) to detect a fatal exception early.
- If a completed task has an exception that is **not** `NoBuildFoundError`, cancel the pending task(s) and re-raise immediately.
- Otherwise, await the remaining task(s) and then apply the existing selection logic.
- Ensure cancellations are awaited/handled so you don’t leak background work.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread ymir/tools/unprivileged/specfile.py Outdated
@nforro

nforro commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread ymir/tools/unprivileged/specfile.py Outdated
@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 04a6d42

@nforro

nforro commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

@qodo-for-packit

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 4b71345

@nforro
nforro force-pushed the release branch 3 times, most recently from e4b50a2 to e05789c Compare August 19, 2026 14:15
Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Sonnet 5 via Claude Code
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.

1 participant