fix: Use organization job routes#833
Conversation
Megh03
left a comment
Megh03
left a comment
There was a problem hiding this comment.
LGTM @lox ! Was just a little curious on the pagination side, like for large orgs with many builds, with the current code . the job lookup could paginate through a lot of pages before finding the job . But I think it's a fair tradeoff to make 🤔
|
Yeah, I don't love the pagination approach, I don't think it will be viable. The challenge is that currently this command doesn't need a pipeline identifier, as the GraphQL endpoint is job UUID only, where as the REST API needs I think realistically we either need to require an optional |
|
Converting back to draft whilst we figure it out. |
|
@buildsworth-bk review please |
![buildsworth-bk-app[bot]](https://avatars.githubusercontent.com/in/3010600?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overall this looks clean and focused — UUID-only commands now work without pipeline context, the helpers in rest.go are small and uniform, and the test coverage exercises path/method/body for each new endpoint. The GraphQL → REST swap for unblock is the most behaviorally significant piece; the isAlreadyUnblocked string match mirrors the existing pattern in cmd/preflight/preflight.go, so it's consistent.
One note on the risk:l1 label: I'd read this closer to L2 than L1. It's not a typo/dep-bump/lockfile change — it switches the verb (GraphQL → REST) for bk job unblock, changes how --data is parsed (must now be a JSON object, previously the raw string was forwarded), and silently drops two previously-functional flags on log and reprioritize. The behavior changes are intentional and tested, but they're the kind of "non-trivial logic" L2 calls for a human sanity-check on rather than AI-only review. Just one Tier-2 question inline.
Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 241, then answer my questions about the findings.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79c57bc938
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Review follow-up: I checked the risk labels and this repo currently only has |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Use PUT /v2/organizations/:org/jobs/:job_id/retry (added in buildkite/buildkite#29824, adopted for other job commands in #833) instead of the build-scoped retry endpoint. This keeps `bk job retry <uuid>` working with only a job UUID and a selected organization — no pipeline or build context, no -p/-b flags, and no behavior change from the GraphQL version it replaces.
3 participants
Description
bk job log,bk job reprioritize, andbk job unblockall take a job UUID, but the REST-backed command paths still required pipeline or build context. That madebk job log <job-uuid>fail withfailed to resolve a pipelineoutside a pipeline checkout, and forcing callers to pass pipeline/build identifiers would change the existing UUID-only command shape.The CLI now calls the new organization-level job REST routes directly under the selected organization. This keeps UUID-only job commands working, avoids scanning builds, and uses the same API surface we added in buildkite/buildkite#29824. The existing
--pipelineand--buildflags onjob logandjob reprioritizeremain accepted for compatibility, but now emit a stderr warning when supplied because they are ignored.Changes
/v2/organizations/:org/jobs/:job_id/{log,reprioritize,unblock}.bk job logto fetch logs directly by job UUID.bk job reprioritizeto reprioritize directly by job UUID.bk job unblockto unblock directly by job UUID while preserving--dataand stdin fields.--pipelineor--buildcontext flags are supplied to UUID-only job commands.Relevant examples now work without pipeline or build context:
Testing
go test ./...)go fmt ./...)Additional checks run:
mise exec -- go test ./cmd/jobmise exec -- go test ./cmd/job ./internal/pipeline/resolver ./internal/build/resolvermise run buildmise run testmise run lint./dist/bk job log 019e6209-ac2a-4d32-84c7-2abcee10099c --no-pager./dist/bk job unblock 019e6209-ac2a-4d32-84c7-2abcee10099c --no-pagerreturned400 This job type cannot be unblocked./dist/bk job reprioritize 019e6209-ac2a-4d32-84c7-000000000000 10 --no-pagerreturned404 No job foundDisclosures / Credits
Codex implemented and validated this update with human direction. The organization-level REST API routes were added separately in buildkite/buildkite#29824.