job_runs: wait for run completion, expose output, dedupe deploy retries#5882
Open
radakam wants to merge 26 commits into
Open
job_runs: wait for run completion, expose output, dedupe deploy retries#5882radakam wants to merge 26 commits into
radakam wants to merge 26 commits into
Conversation
Collaborator
Integration test reportCommit: 38e0761
9 interesting tests: 4 RECOVERED, 4 SKIP, 1 flaky
Top 4 slowest tests (at least 2 minutes):
|
radakam
force-pushed
the
job-runs-idempotency-wait-for-completion
branch
from
July 13, 2026 09:13
acf313b to
3a5ea16
Compare
Job runs now send an automatic idempotency_token on run-now so a retried deployment reuses the existing run instead of triggering a duplicate. The token is the hex SHA-256 of the RunNow request (job_id cleared of the token itself), computed in DoCreate on a copy so it is never persisted to state. A user-set idempotency_token is rejected at validation time, and the local testserver now deduplicates run-now by token so the behavior is covered by tests.
Covers the idempotency token end-to-end: deploy triggers a run, the run is dropped from local state (keeping the job so job_id and the derived token stay stable), and a redeploy re-issues run-now. The backend dedupes on the token and returns the existing run, so no duplicate is created. Drop the DoCreate-level TestJobRunIdempotentCreate since the acceptance test now covers that path end-to-end; the token helper's unit tests remain.
Add WaitAfterCreate to the job_runs direct-engine resource: it polls
until the run reaches a terminal state and returns the run's output
fields (state, run_id, run_page_url, ...) so downstream resources can
read e.g. ${resources.job_runs.x.state.result_state}. A FAILED result
is surfaced as readable state rather than failing the deploy; only
INTERNAL_ERROR fails it.
Add the wait_output acceptance test (a downstream job reads result_state,
and a redeploy stays a no-op so the resolved value doesn't churn), and
extend idempotent_recreate to cover both mid-run retry paths: a lost run
id (token dedupes run-now) and a recorded run id (retry is a no-op).
Regenerate the redeploy golden, which now shows the run TERMINATED.
radakam
force-pushed
the
job-runs-idempotency-wait-for-completion
branch
from
July 14, 2026 10:07
51dca78 to
66ca15a
Compare
Add a changelog fragment for the idempotency + wait-for-completion work. The shared CRUD harness compared WaitAfterCreate's result against the read taken right after DoCreate. A job run transitions RUNNING -> TERMINATED while we wait, so compare against a fresh read taken after the wait instead, mirroring how DoUpdate is already validated.
Contributor
Approval status: pending
|
Derive the idempotency token from the whole JobRunState so identity fields added later join dedup automatically. Clean up the fake's token map on run delete so a recreate starts a fresh run. Narrow the golden mask to the idempotency_token field only. Add unit tests covering WaitAfterCreate on FAILED (surfaced as state) and INTERNAL_ERROR (fails the deploy).
An empty `job_runs.<name>:` entry unmarshals to a nil *resources.JobRun (convert.ToTyped), so dereferencing jr.IdempotencyToken would panic. Skip nil entries and add a regression test.
The SDK's default RunNow waiter caps the wait at 20 minutes, so a legitimately long run (schema migration, model training) would fail the deploy even though the run itself is healthy. Raise the WaitAfterCreate budget to 24h, matching the budget `bundle run` already uses to wait on a job run, so deploy blocks until the run is ready or failed while still guaranteeing an unattended (CI) deploy can't hang forever.
The Jobs API does not free an idempotency_token when its run is deleted: reusing the token then returns an error instead of starting a fresh run. Confirmed against a real workspace (HTTP 400, error_code BAD_REQUEST). Keep the token->run mapping as a tombstone on delete and error on reuse so the fake matches documented behavior.
A run is an immutable historical record, so destroy/recreate now leave it in place (the design doc specifies DoDelete -> noop). This also avoids tombstoning the deterministic idempotency_token: the Jobs API does not free a token when its run is deleted, so deleting would break re-running the same config after destroy or a value flip-back. Add acceptance tests for a FAILED run being surfaced (not fatal) and for destroy -> redeploy, and tighten comments across the resource, validator, and testserver.
Surface the run page URL before WaitAfterCreate blocks so a long (up to 24h) wait isn't opaque. Add an acceptance test for the dev->prod->dev flip-back (the noop delete keeps the original run alive, so its token dedupes back to it) and a testserver unit test asserting a deleted idempotency_token is tombstoned.
radakam
changed the title
The token was a hash of only the run config, so two distinct job_run resources with byte-identical config produced the same token and the second run-now deduped onto the first, collapsing two intended runs into one. Include the deployment-scoped resource key in the hash: distinct resources now get distinct tokens (both run), a config change still re-triggers, and an unchanged retry still dedupes. The key is threaded via a framework-owned context accessor (WithResourceIdentity) so it stays a cross-cutting deploy concern rather than a per-resource hack.
WaitAfterCreate previously let any TERMINATED/SKIPPED run pass, surfacing a FAILED result as readable state instead of a deploy failure. Now only a SUCCESS result_state completes the deploy; every other terminal outcome (FAILED, TIMEDOUT, CANCELED, SUCCESS_WITH_FAILURES, SKIPPED, ...) returns an error. INTERNAL_ERROR and timeout are still handled by the waiter. Rewrites the failed_run acceptance test to assert the deploy aborts (and dependents are not created) and updates the unit tests accordingly.
…wait-for-completion # Conflicts: # bundle/direct/dresources/all_test.go
Move the terminal-state wait from WaitAfterCreate into DoCreate so a deploy interrupted mid-wait re-establishes the wait on resume. The framework persists state only after DoCreate returns, so an interrupted wait saves nothing and the next deploy re-triggers RunNow; the idempotency_token rejoins the same run and waits again, instead of the planner seeing the run as up-to-date and skipping the wait. A run whose wait fails is therefore no longer written to state, so the failed_run acceptance output drops the "waiting after creating" prefix and no longer lists the run for deletion on destroy.
The idempotency_token is a content hash, so identical config never re-runs and a run that finished unsuccessfully can't be retried by redeploying. Add an optional `rerun` field folded into the token (never sent to the API); changing it recreates the run. Point the idempotency_token validator at it. Also tighten job_run comments and regenerate the bundle schema.
- Set Rerun in RemapState's JobRunState literal (exhaustruct lint). - Regenerate acceptance/bundle/refschema/out.fields.txt for the new rerun field.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Milestone 2 for the
job_runsresource: reliable deploys with usable output.DoCreatetriggers the run viarun-nowand then blocks until it reaches a terminal state, printing the run page URL while it waits. A deploy finishes only once the run does.SUCCESS. Any other terminal outcome (FAILED,TIMEDOUT,CANCELED,SUCCESS_WITH_FAILURES,SKIPPED,INTERNAL_ERROR) fails the deploy with the run's state message, so dependents never build on a run that didn't succeed.DoReadreturns the fullGetRunview, so dependents can reference${resources.job_runs.<name>.state.result_state},run_id,run_page_url, etc.run-nowuses an auto-computedidempotency_token, so a crash betweenrun-nowand saving the id doesn't start a duplicate — the re-triggeredrun-nowrejoins the same run.rerunfield is the supported way to force a fresh run of otherwise-unchanged config, or to retry a run that finished unsuccessfully. It is folded into the token but never sent to the API.DoDeleteis a noop. Runs are left in place on destroy/recreate.Key design decisions
Wait lives inside
DoCreate, notWaitAfterCreate. The framework persists state only afterDoCreatereturns, so a deploy interrupted mid-wait saves nothing and the next deploy re-triggersrun-now. Theidempotency_tokenmakes that re-trigger rejoin the same run, so the wait is re-established instead of skipped. Persisting the run before the wait (asWaitAfterCreatewould) lets the planner see the run as up-to-date and skip the wait on resume — so the wait is deliberately kept inDoCreate(follows the rationale of #5391).idempotency_token= SHA-256 of the resource key + desired state, computed automatically. Seeding with the resource key means two resources with byte-identical config get distinct tokens, so each triggers its own run. Hashing the wholeJobRunStatemeans milestone-3 triggers /depends_onjoin the hash for free —on_bundle_deployetc. will re-fire runs with no special casing. Stripped from state to keep plans clean.idempotency_tokenstays computed;rerunis the user knob. The token is derived, so a user-set value would have no effect — a validator rejects it and points atrerun. Because a pure content hash otherwise conflates "resume the same attempt" with "start a new one," the optionalrerunfield is mixed into the hash (and into the diffed state, so a change recreates the run) but never reaches the API. Leaving it unset preserves crash-recovery dedup; bumping it is the one supported way to re-run identical config or retry a run that ended non-SUCCESS.DoDeleteis a noop; runs are never deleted. A run is an immutable historical record. Crucially, the Jobs API does not free a token when its run is deleted, so deleting would tombstone the deterministic token and break re-running the same config afterdestroyor a value flip-back (dev → prod → dev). Leaving the run in place keeps the token a pure reliability aid — it can only dedupe, never fail.24h wait budget. Large enough for a legitimate long run (migration, training), bounded so an unattended CI deploy can't hang forever. Matches
bundle run.Why
Triggering a job at deploy today needs custom scripts outside the bundle. This makes it declarative (migrations, training, data prep) and lets the same deploy consume the run's output.
rerunkeeps it usable in practice: re-running unchanged config and retrying a failed run stay one-line config changes rather than dead ends.Tests
idempotent_recreate(retry dedupes),distinct_runs(two identical configs trigger separate runs),wait_output(downstream readsresult_state),failed_run(aFAILEDrun fails the deploy; dependents report "dependency failed"),destroy_redeploy(run left in place, redeploy re-runs with no tombstone error),flip_back(dev → prod → devdedupes back to the original run),rerun(bumpingreruntriggers a new run while identical config dedupes, andrerunnever appears in therun-nowbody), plusbasic,redeploy,job_parameters.idempotencyTokenstability/rejection, resource-key seeding, andrerunsensitivity;DoCreatesurfaces non-SUCCESSoutcomes / fails onINTERNAL_ERROR/ polls pastRUNNINGuntil terminal;ValidateJobRunIdempotencyToken.run-nowdedup + deleted-token tombstone.