TRT-2735: Move PayloadForJobRun from BigQuery to PostgreSQL#3690
TRT-2735: Move PayloadForJobRun from BigQuery to PostgreSQL#3690mstaeble wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
@mstaeble: This pull request references TRT-2735 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 story 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. |
|
Skipping CI for Draft Pull Request. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe job run payload endpoint now reads payload data from the local database instead of BigQuery. The payload type is JSON-oriented, the BigQuery query label constant is removed, and the endpoint now requires local DB capability. ChangesLocal DB job run payload flow
Sequence Diagram(s)sequenceDiagram
participant Serve
participant jsonJobRunPayload
participant PayloadForJobRun as api.PayloadForJobRun
participant DB as db.DB
Serve->>jsonJobRunPayload: request to /api/job/run/payload
jsonJobRunPayload->>PayloadForJobRun: jobRunIDStr
PayloadForJobRun->>DB: query release_job_runs joined with release_tags
DB-->>PayloadForJobRun: rows or res.Error
PayloadForJobRun-->>jsonJobRunPayload: []apitype.JobPayload
jsonJobRunPayload-->>Serve: JSON payload response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✨ Finishing Touches🧪 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mstaeble 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 |
|
Scheduling required tests: |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@pkg/sippyserver/server.go`:
- Line 1364: The job-run payload lookup is missing request cancellation
propagation, so the database query cannot stop on canceled or timed-out
requests. Update the call site in server.go to pass req.Context() into
PayloadForJobRun, and update PayloadForJobRun to accept a context.Context and
use GORM’s WithContext on the query path so the lookup honors request
cancellation.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3d667704-38bb-4126-aa44-275a1ea70a23
📒 Files selected for processing (4)
pkg/api/releases.gopkg/apis/api/types.gopkg/bigquery/bqlabel/labels.gopkg/sippyserver/server.go
💤 Files with no reviewable changes (1)
- pkg/bigquery/bqlabel/labels.go
Query release_job_runs JOIN release_tags instead of BigQuery's ci_analysis_us.jobs table. The release_job_runs table already has a unique index on prow_job_run_id, so lookups are fast and no schema migration is needed. Also removes the BigQuery dependency (bq.NullString) from the JobPayload API type in favor of *string, and changes the endpoint capability from ComponentReadinessCapability to LocalDBCapability. TRT-2735 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
aa341d4 to
e8eb42c
Compare
|
Scheduling required tests: |
|
@mstaeble: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Summary
PayloadForJobRunwith a PostgreSQL query joiningrelease_job_runsandrelease_tags. Therelease_job_runstable already has a unique index onprow_job_run_id, so no schema migration is needed.bq.NullStringdependency from theJobPayloadAPI type, replacing it with*string.ComponentReadinessCapabilitytoLocalDBCapability, making the endpoint available without BigQuery credentials.Fixes https://issues.redhat.com/browse/TRT-2735
Test plan
go vet ./pkg/...passesgo build ./pkg/...passesgo testpasses for all affected packagesStaging verification
Ran
sippy serveagainst the staging database and tested the/api/job/run/payloadendpoint:🤖 Generated with Claude Code
Summary by CodeRabbit
/api/job/run/payloadendpoint access now follows the local database capability check, which may change which clients can retrieve this data.