Store and consult denylist for repeated failures - #65
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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. 📝 WalkthroughWalkthroughChangesFailed-build denylist workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RebuildBatch
participant db_models
participant failed_builds
participant batching
RebuildBatch->>db_models: Query failed SCM URLs
db_models->>failed_builds: Read denylist in batches
failed_builds-->>db_models: Return denylisted URLs
db_models-->>RebuildBatch: Filter eligible triggers
RebuildBatch->>batching: Skip empty batch
RebuildBatch->>db_models: Record failed SCM URLs
db_models->>failed_builds: Insert unique URLs
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
elnbuildsync/rebuildbatch.py (1)
226-233: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep task failure metadata separate from SCM URLs.
RebuildBatchSlice.run()can return"Task: {task_id}"orNonealongside URL strings.RebuildBatch.run()treats every entry as an SCM URL. The task label can enter the denylist as a fake URL, andNonecan break email formatting. Preserve the structured failure data and pass only valid SCM URLs to URL-based consumers.🤖 Prompt for 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. In `@elnbuildsync/rebuildbatch.py` around lines 226 - 233, Update RebuildBatch.run() to preserve the structured failure metadata returned by RebuildBatchSlice.run() while filtering all_successes and all_failures before URL-based consumers. Exclude None and task labels such as “Task: {task_id}” from record_failed_build_urls and any URL/email formatting, passing only valid SCM URL strings; retain task failure information separately for its intended consumers.Source: Learnings
🤖 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/rebuildbatch.py`:
- Around line 79-98: Update the SCM lookup handling in the loop around
BuildTrigger.get_scmurl(): treat InfoUnavailableError as the isolated
missing-URL case, but stop completing triggers for that or other lookup
failures. Propagate failed Koji requests out of the loop so
process_message_batch() leaves queued triggers uncompleted for retry, while
preserving successful triggers in triggers_with_scmurl.
In `@README.md`:
- Around line 39-43: Fix the malformed Markdown table rows in README.md: lines
39-43, keep the Denylist entry within one table row or use <br> inside its
second cell; lines 93-101, apply the same formatting to every When row. Ensure
no continuation text remains outside the table cells.
---
Outside diff comments:
In `@elnbuildsync/rebuildbatch.py`:
- Around line 226-233: Update RebuildBatch.run() to preserve the structured
failure metadata returned by RebuildBatchSlice.run() while filtering
all_successes and all_failures before URL-based consumers. Exclude None and task
labels such as “Task: {task_id}” from record_failed_build_urls and any URL/email
formatting, passing only valid SCM URL strings; retain task failure information
separately for its intended consumers.
🪄 Autofix
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: 4bd796c7-db79-43c4-b58d-85b9a658c878
📒 Files selected for processing (5)
README.mdelnbuildsync/batching.pyelnbuildsync/buildtrigger.pyelnbuildsync/db_models.pyelnbuildsync/rebuildbatch.py
Once a build (identified by its SCM URL) has been part of a batch and has failed, save it to the database in a denylist. When starting up a batch, check each requested build to see if it matches the denylist and exclude it if it does. This will avoid endless retries in the periodic catch-up task. Signed-off-by: Stephen Gallagher <sgallagh@redhat.com>
Signed-off-by: Stephen Gallagher <sgallagh@redhat.com>
| level=logging.WARNING, | ||
| ) | ||
| except Exception: | ||
| logger.exception( |
bhoy-troy
left a comment
bhoy-troy
left a comment
There was a problem hiding this comment.
Looks good!
Just a question re potential double logging.
2 participants
Once a build (identified by its SCM URL) has been part of a batch and has failed, save it to the database in a denylist. When starting up a batch, check each requested build to see if it matches the denylist and exclude it if it does.
This will avoid endless retries in the periodic catch-up task.
Fixes: #64
Summary by CodeRabbit
New Features
Bug Fixes
Documentation