Skip to content

Handle successful builds awaiting signing - #52

Merged
sgallagher merged 1 commit into
fedora-eln:mainfrom
sgallagher:signing
Jul 27, 2026
Merged

Handle successful builds awaiting signing#52
sgallagher merged 1 commit into
fedora-eln:mainfrom
sgallagher:signing

Conversation

@sgallagher

@sgallagher sgallagher commented Jul 22, 2026

Copy link
Copy Markdown
Member

Handle successful builds awaiting signing

Now that we wait for Bodhi updates to reach stable, there's a new stage
in the process that we need to account for on the status page. If a
build is tagged with *-signing-pending, we will assign it to the SIGNING
status, rather than treating it as FAILED.

Fixes: #50

Signed-off-by: Stephen Gallagher sgallagh@redhat.com

Summary by CodeRabbit

  • New Features

    • Added a SIGNING build status for builds awaiting signing.
    • Status details now identify completed builds that are pending signing or missing expected tagging.
    • Build status pages now select the latest build for each package.
  • Style

    • Added warning-colored styling for signing statuses in the status table.
  • Bug Fixes

    • Improved asynchronous Koji request handling and status updates.

@sgallagher
sgallagher requested a review from bhoy-troy July 22, 2026 16:16
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@sgallagher, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 98868a9e-ab34-4a06-9549-b2d9d9944b5f

📥 Commits

Reviewing files that changed from the base of the PR and between 2cba8f2 and 6edb954.

📒 Files selected for processing (2)
  • elnbuildsync/status.py
  • elnbuildsync/templates/status.html
📝 Walkthrough

Walkthrough

The status page now identifies builds awaiting signing, reports explicit build states, and renders a SIGNING status. Koji calls execute synchronously within Twisted worker threads, while status computation awaits asynchronous package checks.

Changes

Status and runtime integration

Layer / File(s) Summary
Signing status detection and display
elnbuildsync/status.py, elnbuildsync/templates/status.html
Build selection now keeps the latest build per package; completed builds are checked for -signing-pending tags, and SIGNING, FAILED, CANCELED, and UNKNOWN statuses are mapped to frontend output.
Threaded Koji request execution
elnbuildsync/kojihelpers/connection.py
Koji requests use a synchronous helper through Twisted’s deferToThread, with comments documenting worker-thread retry behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant StatusPage
  participant PackageStatus
  participant Koji
  participant Frontend
  StatusPage->>PackageStatus: await package status
  PackageStatus->>Koji: query tags for completed build
  Koji-->>PackageStatus: return build tags
  PackageStatus-->>StatusPage: return SIGNING or other status
  StatusPage->>Frontend: render status
  Frontend-->>StatusPage: apply status-signing class
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: handling builds awaiting signing.
Linked Issues check ✅ Passed The PR addresses issue #50 by marking signing-pending successful builds as SIGNING instead of FAILED.
Out of Scope Changes check ✅ Passed The extra Koji and status-page refactors support the signing-status feature and do not appear unrelated.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@elnbuildsync/status.py`:
- Around line 239-264: Update the build-status logic so COMPLETE builds are
checked for a “-signing-pending” tag via listTags before branching on
tagged_builds. Reuse the existing BuildStatus.SIGNING and “Build awaiting
signing” handling, ensuring first builds without a stable-tag entry are detected
instead of falling through to “Build is not tagged.”

In `@tests/local_test_daemon.sh`:
- Around line 374-377: Quote each complete volume specification in the Docker
argument list, including the mounts using SCRIPT_DIR and PROJ_DIR, so repository
paths containing spaces remain a single argument. Preserve the existing
container destinations and mount options.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f5830570-2c82-4421-b95b-dbe8f10ed863

📥 Commits

Reviewing files that changed from the base of the PR and between 82051f2 and c5c47e3.

📒 Files selected for processing (7)
  • Dockerfile
  • elnbuildsync/kojihelpers/connection.py
  • elnbuildsync/status.py
  • elnbuildsync/templates/status.html
  • requirements.txt
  • run.sh
  • tests/local_test_daemon.sh
💤 Files with no reviewable changes (1)
  • requirements.txt

Comment thread elnbuildsync/status.py
Comment thread tests/local_test_daemon.sh Outdated
@sgallagher
sgallagher force-pushed the signing branch 2 times, most recently from ebfdd20 to eb6a48b Compare July 22, 2026 17:21

@bhoy-troy bhoy-troy left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good

@sgallagher
sgallagher force-pushed the signing branch 3 times, most recently from 6afe673 to 6bae3bd Compare July 27, 2026 20:02
Now that we wait for Bodhi updates to reach stable, there's a new stage
in the process that we need to account for on the status page. If a
build is tagged with *-signing-pending, we will assign it to the SIGNING
status, rather than treating it as FAILED.

Fixes: fedora-eln#50

Signed-off-by: Stephen Gallagher <sgallagh@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@sgallagher
sgallagher merged commit b315a47 into fedora-eln:main Jul 27, 2026
6 checks passed
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.

Status page reports failures on successful builds

2 participants