fix(#3170): dispatch SRPM status check even when DB already shows success - #3171
fix(#3170): dispatch SRPM status check even when DB already shows success#3171fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
shows success Remove the `srpm_build.status == BuildStatus.pending` guard from the SRPM processing block in `update_copr_builds()`. When `copr_build_end` handlers for individual chroots update the SRPM build DB row to success as a side effect before the babysit task runs, babysit previously skipped the entire SRPM processing block, leaving the SRPM GitHub status check stuck in "pending". Now babysit always calls `update_srpm_build_state()` when an SRPM build exists, regardless of the DB status. This is safe because `update_srpm_build_state()` has its own guard that only proceeds when the Copr API confirms the build ended (succeeded/failed), and the `CoprBuildEndHandler` dispatch is idempotent. The `set_status(BuildStatus.error)` call in the `CoprNoResultException` handler is now guarded to only apply when the SRPM build is still pending, preventing an already-succeeded SRPM build from being incorrectly set to error if the Copr API data has expired. Note: Unit/integration tests could not run in the sandbox due to missing system RPM Python bindings (Fedora-specific). The project uses containerized testing (`make check-in-container`). Manual verification of test suite is required. Closes #3170
|
🤖 Finished Review · ✅ Success · Started 7:57 AM UTC · Completed 8:12 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.23 |
|
✔️ pre-commit SUCCESS in 1m 53s |
ReviewFindingsHigh
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note: The following review comments could not be posted on the diff (GitHub returned 422) and are included here instead:
tests/integration/test_babysit.py(file-level): Line 231 · [high] missing-test
Four existing tests (test_check_copr_build_updated, test_check_copr_build_waiting_started, test_check_copr_build_waiting_already_started, test_check_update_copr_builds_timeout) set up SRPMBuildModel.get_by_copr_build_id returning a mock with status=BuildStatus.success but do not mock build_proxy.get_source_chroot(). Before this PR, the srpm_build.status == BuildStatus.pending guard skipped the SRPM processing block, so get_source_chroot was never called. After removing that guard, the code enters the try block and calls get_source_chroot, which is not registered on the pure flexmock build_proxy. The PR updated test_check_copr_build_already_successful but missed these four tests.
Suggested fix: Add get_source_chroot mocking to the build_proxy flexmock in each of these four tests (matching the pattern in test_check_copr_build_already_successful), and add appropriate expectations for update_srpm_build_state.
2 participants
Summary
srpm_build.status == BuildStatus.pendingguard from the SRPM processing block inupdate_copr_builds(), soupdate_srpm_build_state()is called whenever an SRPM build exists — regardless of DB statusset_status(BuildStatus.error)in theCoprNoResultExceptionhandler to only apply when the SRPM build is still pending, preventing overwriting a valid success/failure statustest_check_copr_build_srpm_status_check_dispatched_when_db_already_successthat verifies the SRPM status check handler is dispatched even when the DB already shows successtest_check_copr_build_already_successfulto account for the new behaviorRoot Cause
When
copr_build_endhandlers for individual chroots ran between babysit retries, they updated the SRPM build's DB row tosuccessas a side effect — but did not send the SRPM's GitHub status check. On the next babysit run, thesrpm_build.status == BuildStatus.pendingguard caused the entire SRPM processing block to be skipped, leaving the SRPM status check stuck in "pending" on GitHub.Why This Is Safe
update_srpm_build_state()already has its own guard (build_copr_srpm.state not in (COPR_SUCC_STATE, COPR_FAIL_STATE)) that only proceeds when the Copr API confirms the SRPM build has ended. TheCoprBuildEndHandlerdispatch via Celery is idempotent — re-sending a status check that was already set is a no-op on GitHub's side.Testing
get_source_chrootAPImake check-in-container) due to Fedora-specific system dependencies (RPM Python bindings)Closes #3170
Post-script verification
agent/3170-babysit-srpm-status-check)ca574c7e4de53a31d6870742f12976f16ad2dd55..HEAD)