TRT-2799: syncPRStatus: Fix SHA mismatch bug (MergeCommitSHA vs Head.SHA)#3821
TRT-2799: syncPRStatus: Fix SHA mismatch bug (MergeCommitSHA vs Head.SHA)#3821openshift-trt-agent[bot] wants to merge 1 commit into
Conversation
syncPRStatus compared MergeCommitSHA (the merge commit GitHub creates) against prow_pull_requests.SHA (the PR head commit from prow job specs). These are different commits so the comparison never matched, leaving merged_at NULL on 82% of rows. Replace the per-PR loop with a batch pipeline that uses Head.SHA from the GitHub PR objects (matching the original correct GetPRSHAMerged behavior). The new implementation queries distinct repos, fetches recently merged PRs via ListRecentlyMergedPRs, then uses CopyToTempTable for a bulk UPDATE and bulk DELETE instead of per-row DB round-trips. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
@openshift-trt-agent[bot]: This pull request references TRT-2799 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: openshift-trt-agent[bot] The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @openshift-trt-agent[bot]. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Summary
Fixes the SHA mismatch bug in
syncPRStatus(TRT-2799).Since January 2023 (commit
d4932de0b),syncPRStatuscomparedMergeCommitSHA(the merge commit GitHub creates when merging a PR) againstprow_pull_requests.SHA(the PR head commit from prow job specs). These are fundamentally different commits and almost never match, somerged_atwas never set via this code path. This left 82% ofprow_pull_requestsrows withmerged_at IS NULL.Changes
pkg/dataloader/prowloader/github/github.go: AddedMergedPRtype andListRecentlyMergedPRsmethod that returns recently merged PRs withHead.SHA(the correct PR head commit SHA) instead ofMergeCommitSHA. Uses the same lazyclosedCachepopulation as the existingIsPrRecentlyMerged.pkg/dataloader/prowloader/prow.go: RewrotesyncPRStatusfrom a per-PR loop with per-row DB saves to a batch pipeline:(org, repo)fromprow_pull_requests WHERE merged_at IS NULLListRecentlyMergedPRsto get merged PRs withHead.SHACOPYall merged PR data to a temp table viaCopyToTempTableUPDATE prow_pull_requests SET merged_atwhereshamatchesDELETE FROM pull_request_commentsfor merged PRs (risk analysis cleanup)pkg/dataloader/prowloader/github/github_test.go: AddedTestClient_ListRecentlyMergedPRsverifying that the method returnsHead.SHA(notMergeCommitSHA), filters out unmerged and headless PRs, and uses the cache on subsequent calls.Test plan
go vet ./pkg/dataloader/prowloader/...passesgo test ./pkg/...passes (all Go unit tests)TestClient_ListRecentlyMergedPRsverifies Head.SHA is returned, not MergeCommitSHAgolangci-lintreports 0 issuesgofmt -wapplied to all changed filesGenerated with Claude Code