Job runs wait for completion - #6091
Open
radakam wants to merge 13 commits into
Open
Conversation
The legacy-to-path run URL conversion is needed outside `bundle run`, so it moves to libs/workspaceurls next to JobRunPath, which it already used. Pure move: the conversion, its doc comment and its tests are unchanged.
The fake workspace reported every run as TERMINATED SUCCESS, overwriting the FAILED state it had just recorded for a task it executed locally. A run now reports the terminal state its tasks add up to, so a failing run can be exercised end to end locally. Tasks whose code the fake workspace does not have are left successful. An immutable deployment, for example, uploads the bundle as a snapshot zip that the fake workspace never unpacks, so there is nothing to execute; that gap is in the fake workspace, not in the job under test. Originally reviewed as #6082.
Deploying a job_run triggered the run and moved on, so a resource referencing the run's outcome saw whatever state the run happened to be in. The resource now implements the framework's WaitAfterCreate hook, which blocks until the run is terminal and republishes the settled remote state; only SUCCESS lets the deploy continue. While it waits, the deploy reports the run page URL and each state change, the way `bundle run` does, since a run can take hours. A run that does not succeed fails the deploy with the failed task, the message that task reported, and a link to the run page. Bounded by 24h, matching `bundle run`. The framework saves the run id before the wait, so a run that fails stays recorded and an unchanged config plans no second run; failed_run covers that. run_page_url is now normalized to the path form that also resolves for non-admins. invariant/configs/job_run.yml.tmpl loses its notebook task, which deploying it would now actually run, and is excluded from cloud runs: a real workspace reports a condition-task-only run as SKIPPED, and a task that does succeed would add a multi-minute cluster run to every variant of a suite that asserts plan and state invariants. Still covered locally.
Collaborator
Integration test reportCommit: ea0a8b3
8 interesting tests: 5 RECOVERED, 3 SKIP
Top 6 slowest tests (at least 2 minutes):
|
The wait reimplemented what `bundle run`'s monitor already does: report the run page URL once, then each state change. Both now go through progress.JobStateTracker, which decides what a poll is worth reporting; the two callers keep their own sinks, since a concurrent deploy reports plain prefixed lines where `bundle run` reports progress events. The failure the deploy reports no longer repeats the run id that the framework's wrapper already carries; what the wait adds is the link to a run that outlives it.
Excluding job_run.yml.tmpl from cloud left job_runs with no cloud coverage at all: every test under resources/job_runs inherits Cloud=false. The behaviour this milestone adds is the one that depends most on the real Jobs API, so wait_cloud triggers a run for real and reads the outcome back out of the downstream job, the way the vector search exclusion points at a dedicated test. Serverless keeps the run to about a minute, and the deploy's progress stream stays out of the golden: a real run reports an unpredictable number of intermediate states.
The ignore_remote_changes for job_parameters assumes GetRun reports every parameter the job defines, not just the ones the run overrode. Assert that on the run wait_cloud already triggers by overriding one of two parameters and planning after the deploy.
Drop the comments that only restate the code, the duplicated note about the framework prefixing the run id, and the filler in the ones that carry a reason.
The context only reached two debug lines about a URL that did not parse, and it had to be threaded through makeJobRunRemote, which is otherwise a pure mapping of the GetRun response.
Reporting every state change made a deploy's output depend on how many states the run passed through, which varies with how long its compute takes to start. That cost the cloud test its coverage: it had to grep the deploy log instead of comparing it. The full state history is still in the log. Also record that a failed run is not run again in the changelog entry.
The message the deploy names a failed task with came from an error the test server writes itself, so nothing checked that a real workspace reports one at all. Assert it does, and that we are not falling back to the states the run reports.
Also fix jobRunServer's doc, which said it returns a server when it returns a client.
radakam
marked this pull request as ready for review
July 29, 2026 14:22
Contributor
Approval status: pending
|
Interrupting a deploy mid-wait leaves the run going, and jobs/runs/delete rejects an active run, so destroy failed and the bundle could not be torn down without cancelling the run by hand. Delete now cancels it first, and waits for the cancellation to settle since the API cancels asynchronously. The interrupt itself was reported as a timeout, blaming the 24h bound for something the user did. It now says it was interrupted, and still links the run, whose page URL is pinned to the first poll that reported one. The run left going is what the next deploy reads, and it triggers no second run, so a reference to the outcome resolves to an empty string. Recorded in a test rather than fixed here: stopping a run the user did not ask to stop is a departure from `bundle run`, which leaves interrupted runs alive.
A real workspace reports a run whose task failed as INTERNAL_ERROR in the deprecated life_cycle_state, though status.state is TERMINATED with termination code RUN_EXECUTION_ERROR. The SDK waiter halts on INTERNAL_ERROR with an error of its own, so the deploy blamed the run for an internal failure instead of naming the task that failed and the message it reported. The wait now polls for any state runIsTerminal accepts, the definition the delete path already used, and leaves the verdict to the run's result. The Jobs API retries a task that failed and reports it once per attempt, so the same task was named twice over. Only its last attempt is reported now. The fake workspace rolls a failed task up to TERMINATED FAILED, so failed_cloud was the only test that saw either of these; both are now covered by unit tests.
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.
Why
The experimental
job_runsresource returned as soon as it triggered a run, so a resource referencing the run's outcome saw a state from a moment afterrun-now, and a run that failed left the deploy reporting success.What
WaitAfterCreateinbundle/direct/dresources/job_run.goblocks until the run is terminal, and onlySUCCESScontinues the deploy. Anything else fails it, naming the outcome, each task that caused the failure with the error that task reported, and a link to the run page. A failed run stays recorded, so the next deploy does not run the job again. While waiting, the user gets the run page URL and then the run's final state; every state change goes to the log, through aprogress.JobStateTrackernow shared withbundle run.Interrupting a deploy mid-wait leaves the run going: destroy now cancels a run that has not finished, since
jobs/runs/deleterejects one, and the error says the wait was interrupted rather than blaming the 24h timeout. That run is also what the next deploy reads, and it triggers no second run, so a reference to the outcome resolves to an empty string; that is recorded in a test rather than fixed, since stopping a run the user did not ask to stop departs frombundle run.Two supporting changes:
JobRunPageURLmoved tolibs/workspaceurlsfor the second caller, and the fake workspace now rolls task outcomes up into the run state so a failing task fails the run — with a task whose code it never uploaded succeeding instead, since that gap is the server's and not the job's.Tests
Unit tests cover success,
FAILED, a named failing task,SKIPPED,INTERNAL_ERROR, an interrupted wait, polling past non-terminal reads, and delete cancelling a run that has not finished.wait_outputandfailed_runcover the deploy against the test server.wait_cloudandfailed_cloudrun the job for real: that the deploy waits, that the parametersGetRunresolves do not read back as drift, and that a real failed task gives us a message rather than our fallback to the run's own states.