Skip to content

Commit 692b289

Browse files
committed
job_runs: report the run page and the outcome, not bundle run's event
The wait borrowed `bundle run`'s progress event to report a run, which put a timestamp in the middle of a deploy's output, made dresources depend on the `bundle run` progress package, and repeated on failure the state message the error already carries. A deploy now prints the run page and, once the run succeeds, its outcome; every state change still goes to the log. bundle/run is untouched again, so the shared JobStateTracker is no longer needed. Review cleanups along with it: - ignore_remote_changes says why each field is ignored rather than calling all of them immutable: job_id is, the parameter fields come back resolved against the job's defaults, and the rest are never returned. The reason is what `bundle plan` shows the user. - Restore the note that `field: ""` matches nothing, which still applies to the root recreate_on_changes rule. - Drop TestLookupReferencePreDeploy_JobRunReferencesAreRaw, left over from the CheckSettled iteration: it asserts framework behaviour this branch does not change. - testCRUD reads the resource again after WaitAfterCreate instead of special casing job_runs, the way the DoUpdate block below it already does. - Drop the RecordRequests failed_run already inherits, and comment the read_id.py call that exists only to register a replacement.
1 parent 2d11b0b commit 692b289

14 files changed

Lines changed: 79 additions & 166 deletions

File tree

acceptance/bundle/resources/job_runs/basic/output.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ Resources:
3535
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/job-runs-basic/default/files...
3636
Deploying resources...
3737
job run [MY_RUN_ID]: Run URL: [DATABRICKS_URL]/jobs/[MY_JOB_ID]/runs/[MY_RUN_ID]?o=[NUMID]
38-
job run [MY_RUN_ID]: [TIMESTAMP] "my-job" TERMINATED SUCCESS
38+
job run [MY_RUN_ID]: SUCCESS
3939
Updating deployment state...
4040
Deployment complete!
4141

acceptance/bundle/resources/job_runs/failed_run/output.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/files...
55
Deploying resources...
66
job run [MY_RUN_ID]: Run URL: [RUN_URL]
7-
job run [MY_RUN_ID]: [TIMESTAMP] "test-job-[UNIQUE_NAME]" INTERNAL_ERROR FAILED Task main failed with message: Workload failed, see run output for details.
87
Error: cannot create resources.job_runs.my_run: waiting after creating id=[MY_RUN_ID]: run did not succeed: FAILED: Task main failed with message: Workload failed, see run output for details.
98
task "main": RuntimeError: intentional failure
109
run page: [RUN_URL]
@@ -28,7 +27,6 @@ Plan: 2 to add, 0 to change, 1 to delete, 1 unchanged
2827
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/files...
2928
Deploying resources...
3029
job run [MY_RUN_ID_2]: Run URL: [RUN_URL]
31-
job run [MY_RUN_ID_2]: [TIMESTAMP] "test-job-[UNIQUE_NAME]" INTERNAL_ERROR FAILED Task main failed with message: Workload failed, see run output for details.
3230
Error: cannot recreate resources.job_runs.my_run: waiting after creating id=[MY_RUN_ID_2]: run did not succeed: FAILED: Task main failed with message: Workload failed, see run output for details.
3331
task "main": RuntimeError: intentional failure
3432
run page: [RUN_URL]

acceptance/bundle/resources/job_runs/failed_run/script

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ title "the failed run is recorded, and not having succeeded is drift"
1818
trace read_id.py my_run
1919
trace $CLI bundle plan
2020

21+
# Name the job too, so the run-now bodies below say which number it is.
2122
read_id.py my_job > /dev/null
2223

2324
# The recreate re-runs the job instead of accepting the recorded failure.

acceptance/bundle/resources/job_runs/failed_run/test.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# job_runs is a direct-engine-only resource; the Terraform provider has no
22
# equivalent, so restrict the matrix to direct.
33
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]
4-
RecordRequests = true
54

65
# Runs the failing job for real, so the message the deploy names the task with is
76
# one a workspace reported rather than one the test server wrote. Serverless needs

acceptance/bundle/resources/job_runs/job_parameters/output.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/job-runs-job-parameters/default/files...
55
Deploying resources...
66
job run [MY_RUN_ID]: Run URL: [DATABRICKS_URL]/jobs/[MY_JOB_ID]/runs/[MY_RUN_ID]?o=[NUMID]
7-
job run [MY_RUN_ID]: [TIMESTAMP] "my-job" TERMINATED SUCCESS
7+
job run [MY_RUN_ID]: SUCCESS
88
Updating deployment state...
99
Deployment complete!
1010

acceptance/bundle/resources/job_runs/redeploy/output.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/job-runs-redeploy/default/files...
55
Deploying resources...
66
job run [MY_RUN_ID]: Run URL: [DATABRICKS_URL]/jobs/[MY_JOB_ID]/runs/[MY_RUN_ID]?o=[NUMID]
7-
job run [MY_RUN_ID]: [TIMESTAMP] "my-job" TERMINATED SUCCESS
7+
job run [MY_RUN_ID]: SUCCESS
88
Updating deployment state...
99
Deployment complete!
1010

@@ -90,7 +90,7 @@ Resources:
9090
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/job-runs-redeploy/default/files...
9191
Deploying resources...
9292
job run [MY_RUN_ID_2]: Run URL: [DATABRICKS_URL]/jobs/[MY_JOB_ID]/runs/[MY_RUN_ID_2]?o=[NUMID]
93-
job run [MY_RUN_ID_2]: [TIMESTAMP] "my-job" TERMINATED SUCCESS
93+
job run [MY_RUN_ID_2]: SUCCESS
9494
Updating deployment state...
9595
Deployment complete!
9696

acceptance/bundle/resources/job_runs/wait/output.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/files...
55
Deploying resources...
66
job run [MY_RUN_ID]: Run URL: [RUN_URL]
7-
job run [MY_RUN_ID]: [TIMESTAMP] "test-job-[UNIQUE_NAME]" TERMINATED SUCCESS
7+
job run [MY_RUN_ID]: SUCCESS
88
Updating deployment state...
99
Deployment complete!
1010

bundle/direct/bundle_plan_test.go

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -308,41 +308,6 @@ func TestLookupReferencePreDeploy_FinishedJobRun(t *testing.T) {
308308
assert.Equal(t, jobs.RunResultStateSuccess, value)
309309
}
310310

311-
// A resource reads the run the workspace reports. These paths are absent from
312-
// JobRunState, so they resolve from the remote, and the failure-shaped values
313-
// below come back unrewritten. result_state is the exception: it resolves to the
314-
// outcome PrepareState requires, which TestJobRunOutcomeIsDrift keeps from
315-
// standing in for one the run never reached.
316-
func TestLookupReferencePreDeploy_JobRunReferencesAreRaw(t *testing.T) {
317-
b := bundleWithSkippedJobRun(t, &dresources.JobRunRemote{
318-
RunId: 123,
319-
RunName: "my-job",
320-
RunPageUrl: "https://myworkspace.databricks.test/jobs/456/runs/123",
321-
ResultState: jobs.RunResultStateFailed,
322-
State: &jobs.RunState{
323-
LifeCycleState: jobs.RunLifeCycleStateTerminated,
324-
ResultState: jobs.RunResultStateFailed,
325-
StateMessage: "task main failed",
326-
},
327-
})
328-
329-
for field, want := range map[string]any{
330-
"state.result_state": jobs.RunResultStateFailed,
331-
"state.life_cycle_state": jobs.RunLifeCycleStateTerminated,
332-
"state.state_message": "task main failed",
333-
"run_id": int64(123),
334-
"run_name": "my-job",
335-
"run_page_url": "https://myworkspace.databricks.test/jobs/456/runs/123",
336-
} {
337-
t.Run(field, func(t *testing.T) {
338-
value, err := b.LookupReferencePreDeploy(t.Context(), structpath.MustParsePath(jobRunKey+"."+field))
339-
340-
require.NoError(t, err)
341-
assert.Equal(t, want, value)
342-
})
343-
}
344-
}
345-
346311
// References are served from the remote state cache only for a run the plan
347312
// skips, and any outcome other than the required SUCCESS is a recreate.
348313
func TestJobRunOutcomeIsDrift(t *testing.T) {

bundle/direct/dresources/all_test.go

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,19 +1000,18 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
10001000
"unexpected differences between remappedState and remappedRemoteStateFromCreate")
10011001
}
10021002

1003-
// Only a job run's state moves while WaitAfterCreate blocks, and its result_state
1004-
// fills in only then, so its field checks below need the settled state.
1005-
waitSettlesState := group == "job_runs"
1006-
10071003
remoteStateFromWaitCreate, err := adapter.WaitAfterCreate(ctx, createdID, newState)
10081004
require.NoError(t, err)
10091005
if remoteStateFromWaitCreate != nil {
1010-
if waitSettlesState {
1011-
remappedState, err = adapter.RemapState(remoteStateFromWaitCreate)
1012-
require.NoError(t, err)
1013-
} else {
1014-
require.Equal(t, remote, remoteStateFromWaitCreate)
1015-
}
1006+
// The resource can move while the wait blocks (a job run only settles
1007+
// then), so the field checks below use what the wait reports. It must
1008+
// match a read taken right after it.
1009+
remotePostWait, err := adapter.DoRead(ctx, createdID)
1010+
require.NoError(t, err)
1011+
require.Equal(t, remotePostWait, remoteStateFromWaitCreate)
1012+
1013+
remappedState, err = adapter.RemapState(remoteStateFromWaitCreate)
1014+
require.NoError(t, err)
10161015
}
10171016

10181017
if adapter.HasDoUpdate() {

bundle/direct/dresources/job_run.go

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"time"
1111

1212
"github.com/databricks/cli/bundle/config/resources"
13-
"github.com/databricks/cli/bundle/run/progress"
1413
"github.com/databricks/cli/libs/cmdio"
1514
"github.com/databricks/cli/libs/log"
1615
"github.com/databricks/cli/libs/workspaceurls"
@@ -168,10 +167,13 @@ func (r *ResourceJobRun) WaitAfterCreate(ctx context.Context, id string, _ *JobR
168167
return nil, err
169168
}
170169

171-
// A run can take hours, so report progress like `bundle run` does. pageURL
170+
// A run can take hours, so report the run page as soon as it is known. pageURL
172171
// outlives the poll so an abandoned wait can still link the run.
173-
var tracker progress.JobStateTracker
174172
var pageURL string
173+
// Every state the run passes through is logged, but only its outcome is
174+
// reported: how many states a run passes through varies with how long its
175+
// compute takes to start, and a deploy's output has to stay reproducible.
176+
var logged jobs.RunState
175177
// Polled here rather than through the SDK waiter, which halts with an error of
176178
// its own on the INTERNAL_ERROR a run whose task failed reports, hiding the
177179
// task that failed.
@@ -182,8 +184,14 @@ func (r *ResourceJobRun) WaitAfterCreate(ctx context.Context, id string, _ *JobR
182184
if err != nil {
183185
return nil, retries.Halt(err)
184186
}
185-
pageURL = cmp.Or(pageURL, run.RunPageUrl)
186-
logRunProgress(ctx, run, &tracker)
187+
if pageURL == "" && run.RunPageUrl != "" {
188+
pageURL = run.RunPageUrl
189+
reportRunLine(ctx, runID, "Run URL: "+workspaceurls.ModernizeJobRunPageURL(pageURL))
190+
}
191+
if run.State.LifeCycleState != logged.LifeCycleState || run.State.ResultState != logged.ResultState {
192+
logged = *run.State
193+
log.Info(ctx, strings.TrimSpace(fmt.Sprintf("job run %d: %s %s", runID, logged.LifeCycleState, logged.ResultState)))
194+
}
187195
if !runIsTerminal(run.State.LifeCycleState) {
188196
return nil, retries.Continues(run.State.StateMessage)
189197
}
@@ -199,8 +207,10 @@ func (r *ResourceJobRun) WaitAfterCreate(ctx context.Context, id string, _ *JobR
199207
return nil, fmt.Errorf("%w%s", err, runPageLine(pageURL))
200208
}
201209
if run.State.ResultState != jobs.RunResultStateSuccess {
210+
// The error names the outcome, so it is not reported twice.
202211
return nil, r.runFailedError(ctx, run)
203212
}
213+
reportRunLine(ctx, runID, string(run.State.ResultState))
204214
return makeJobRunRemote(run), nil
205215
}
206216

@@ -284,26 +294,6 @@ func runPageLine(rawURL string) string {
284294
return "\nrun page: " + workspaceurls.ModernizeJobRunPageURL(rawURL)
285295
}
286296

287-
// logRunProgress logs every state change like `bundle run` does, but reports only
288-
// the run page URL and the final state to the user: how many states a run passes
289-
// through varies with how long its compute takes to start, and a deploy's output
290-
// has to stay reproducible.
291-
func logRunProgress(ctx context.Context, run *jobs.Run, tracker *progress.JobStateTracker) {
292-
event, first := tracker.Poll(run)
293-
if event == nil {
294-
return
295-
}
296-
log.Info(ctx, event.String())
297-
if first && run.RunPageUrl != "" {
298-
line := "Run URL: " + workspaceurls.ModernizeJobRunPageURL(run.RunPageUrl)
299-
log.Info(ctx, line)
300-
reportRunLine(ctx, run.RunId, line)
301-
}
302-
if runIsTerminal(run.State.LifeCycleState) {
303-
reportRunLine(ctx, run.RunId, event.String())
304-
}
305-
}
306-
307297
// runIsTerminal reports whether a run has stopped, whatever it stopped as.
308298
func runIsTerminal(state jobs.RunLifeCycleState) bool {
309299
return state == jobs.RunLifeCycleStateTerminated ||

0 commit comments

Comments
 (0)