feat(api): add include=last_run to job reads - #1
Conversation
current_run only ever matches a running attempt, so pending-after-retry and terminal jobs carry no run data at all — consumers can't show where a failed attempt got to, or read the partial result a handler returned alongside its error. Add a last_run include that returns the newest attempt whatever its status, batched with DISTINCT ON so a jobs-list expansion stays one query. ?include= now parses as a comma-separated set, ignoring unknown values, so current_run keeps working unchanged and both can be asked for at once. last_run is omitempty, leaving existing current_run responses byte-identical. SSE serves both from a single LastRun lookup: the newest attempt is the in-flight one whenever a job is running, so current_run keeps its exact meaning while last_run also survives terminalization.
![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: b45acae9f4
ℹ️ 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".
| if err != nil { | ||
| c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | ||
| return | ||
| out := jobWithRun{Job: job} |
There was a problem hiding this comment.
For GET /jobs/:id?include=last_run, constructing jobWithRun here leaves CurrentRun nil, but its JSON field lacks omitempty, so the response unexpectedly contains "current_run": null; the list path has the same behavior. This breaks the new set-like include semantics and prevents clients from distinguishing an unrequested expansion from a requested current run that does not exist. Use conditional response shapes or marshaling so current_run remains present and nullable only when current_run was requested.
Useful? React with 👍 / 👎.
1 participant
service.LastRun(jobID)— newest attempt whatever its status, nil when never claimedservice.LastRunsForJobs(ids)— batched viaDISTINCT ON (job_id) ... ORDER BY job_id, attempt DESC, one query per list?include=as a comma-separated set inGetJobandListJobs; unknown values ignored so adding more later can't break existing clientslast_runto thejobWithRunresponse asomitempty, soinclude=current_runresponses stay byte-identicalbuildJobEventnow serves both fields from oneLastRunlookup — the newest attempt is the in-flight one whenever a job is running, socurrent_runkeeps its exact meaningJob.LastRunfield andJobsFilter.IncludeLastRun, which builds the comma-separated paramCurrentRuncan't serve: survives a failed attempt, returns the highest attempt, one row per job, absent when never claimedWhy
CurrentRunfiltersstatus = 'running'(service/job.go:504,:665), and run/job transitions are atomic —Fail/Completeare transactional and the reaper is a single CTE. Socurrent_runis non-null iff the job is active, leaving pending-after-retry and terminal jobs with no run data.That means consumers can't see where a failed attempt got to, or read a partial result a handler attached to a failure via
FailRequest.Result. The Mapache dashboard's jobs list has dead code built on the assumption that this already worked.Notes
GetJob's Go-client signature still takes a singleincludeCurrentRun bool. Left alone to stay source-compatible; worth widening if a Go consumer needslast_runfrom a single-job read.gofmt -lflagsapi/job.go, which it also does on cleanmain(the comment insidefailRequestsplits the struct's alignment group). Not touched here.Test plan
go test ./...green, including 5 new cases against real Postgres via testcontainersgo build ./...andgo vet ./...clean