Skip to content

Commit 27f8a9b

Browse files
authored
Improve DLT UX (#1474)
## Changes - Load events with `retry` instead of `setTimeout` - Better handle cancellation - Show spinning icon for more DLT states - Merge Event Log and Run Status tree sections - Remove run "cause" item, since the same thing is visible in one of the events <img width="602" alt="Screenshot 2024-11-28 at 16 24 04" src="https://github.com/user-attachments/assets/0481ca7e-d00d-4256-99ff-192fd59f696a"> ## Tests <!-- How is this tested? -->
1 parent ba9518d commit 27f8a9b

File tree

11 files changed

+186
-264
lines changed

11 files changed

+186
-264
lines changed

packages/databricks-vscode/src/bundle/BundlePipelinesManager.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -570,9 +570,10 @@ export async function locationToRange(
570570
}
571571

572572
// Cell URIs are private and there is no public API to generate them.
573-
// Here we generate a URI for a cell in the same way as VS Code does it, but without appending base64 schema to it (vsode can still parse such uris).
573+
// Here we generate a URI for a cell in the same way as VS Code does it internally.
574574
// https://github.com/microsoft/vscode/blob/9508be851891834c4036da28461824c664dfa2c0/src/vs/workbench/services/notebook/common/notebookDocumentService.ts#L45C41-L45C47
575-
// As an alternative we can access these URIs by relying on open notebook editors, which means you won't get diagnostics in the problems panel unless you open a notebook.
575+
// As an alternative we can access these URIs by relying on open notebook editors,
576+
// which means you won't get diagnostics in the problems panel unless you open a notebook.
576577
// (Which is how it actually is for disgnostics that python extension provides)
577578
function generateNotebookCellURI(notebook: Uri, handle: number): Uri {
578579
const lengths = ["W", "X", "Y", "Z", "a", "b", "c", "d", "e", "f"];

packages/databricks-vscode/src/bundle/run/JobRunStatus.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export class JobRunStatus extends BundleRunStatus {
7272
return;
7373
}
7474

75+
this.runState = "cancelling";
7576
const client = await this.authProvider.getWorkspaceClient();
7677
await (
7778
await client.jobs.cancelRun({run_id: parseInt(this.runId)})

packages/databricks-vscode/src/bundle/run/PipelineRunStatus.ts

Lines changed: 76 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,19 @@
22
import {BundleRunStatus} from "./BundleRunStatus";
33
import {AuthProvider} from "../../configuration/auth/AuthProvider";
44
import {onError} from "../../utils/onErrorDecorator";
5-
import {pipelines, WorkspaceClient} from "@databricks/databricks-sdk";
5+
import {
6+
logging,
7+
pipelines,
8+
retry,
9+
Time,
10+
TimeUnits,
11+
WorkspaceClient,
12+
} from "@databricks/databricks-sdk";
13+
import {
14+
LinearRetryPolicy,
15+
RetriableError,
16+
} from "@databricks/databricks-sdk/dist/retries/retries";
17+
import {Loggers} from "../../logger";
618

719
function isRunning(status?: pipelines.UpdateInfoState) {
820
if (status === undefined) {
@@ -16,7 +28,7 @@ export class PipelineRunStatus extends BundleRunStatus {
1628
public data: pipelines.UpdateInfo | undefined;
1729
public events: pipelines.PipelineEvent[] | undefined;
1830

19-
private interval?: NodeJS.Timeout;
31+
private logger = logging.NamedLogger.getOrCreate(Loggers.Extension);
2032

2133
constructor(
2234
private readonly authProvider: AuthProvider,
@@ -47,43 +59,53 @@ export class PipelineRunStatus extends BundleRunStatus {
4759
return;
4860
}
4961

50-
if (this.runId === undefined) {
62+
const runId = this.runId;
63+
if (runId === undefined) {
5164
throw new Error("No update id");
5265
}
5366

54-
const client = await this.authProvider.getWorkspaceClient();
5567
this.runState = "running";
5668

57-
this.interval = setInterval(async () => {
58-
try {
59-
if (this.runId === undefined) {
60-
throw new Error("No update id");
61-
}
62-
const getUpdateResponse = await client.pipelines.getUpdate({
63-
pipeline_id: this.pipelineId,
64-
update_id: this.runId,
65-
});
66-
this.data = getUpdateResponse.update;
67-
68-
if (this.data?.creation_time !== undefined) {
69-
this.events = await this.fetchUpdateEvents(
70-
client,
71-
this.data?.creation_time,
72-
this.data?.update_id
73-
);
74-
}
69+
try {
70+
await retry({
71+
timeout: new Time(48, TimeUnits.hours),
72+
retryPolicy: new LinearRetryPolicy(
73+
new Time(5, TimeUnits.seconds)
74+
),
75+
fn: async () => {
76+
if (this.runState !== "running") {
77+
return;
78+
}
79+
await this.updateRunData(runId);
80+
if (isRunning(this.data?.state)) {
81+
throw new RetriableError();
82+
} else {
83+
this.runState = "completed";
84+
}
85+
},
86+
});
87+
} catch (e) {
88+
this.runState = "error";
89+
throw e;
90+
}
91+
}
7592

76-
// If update is completed, we stop polling.
77-
if (!isRunning(this.data?.state)) {
78-
this.markCompleted();
79-
} else {
80-
this.onDidChangeEmitter.fire();
81-
}
82-
} catch (e) {
83-
this.runState = "error";
84-
throw e;
85-
}
86-
}, 5_000);
93+
private async updateRunData(runId: string) {
94+
const client = await this.authProvider.getWorkspaceClient();
95+
const getUpdateResponse = await client.pipelines.getUpdate({
96+
pipeline_id: this.pipelineId,
97+
update_id: runId,
98+
});
99+
this.data = getUpdateResponse.update;
100+
this.onDidChangeEmitter.fire();
101+
if (this.data?.creation_time !== undefined) {
102+
this.events = await this.fetchUpdateEvents(
103+
client,
104+
this.data?.creation_time,
105+
this.data?.update_id
106+
);
107+
this.onDidChangeEmitter.fire();
108+
}
87109
}
88110

89111
private async fetchUpdateEvents(
@@ -106,47 +128,33 @@ export class PipelineRunStatus extends BundleRunStatus {
106128
return events;
107129
}
108130

109-
private markCompleted() {
110-
if (this.interval !== undefined) {
111-
clearInterval(this.interval);
112-
this.interval = undefined;
113-
}
114-
this.runState = "completed";
115-
}
116-
117-
private markCancelled() {
118-
if (this.interval !== undefined) {
119-
clearInterval(this.interval);
120-
this.interval = undefined;
121-
}
122-
this.runState = "cancelled";
123-
}
124-
125131
async cancel() {
126132
if (this.runState !== "running" || this.runId === undefined) {
127-
this.markCancelled();
133+
this.runState = "cancelled";
128134
return;
129135
}
130136

131-
const client = await this.authProvider.getWorkspaceClient();
132-
const update = await client.pipelines.getUpdate({
133-
pipeline_id: this.pipelineId,
134-
update_id: this.runId,
135-
});
136-
// Only stop the pipeline if the tracked update is still running. The stop API stops the
137-
// latest update, which might not be the tracked update.
138-
if (isRunning(update.update?.state)) {
139-
await (
140-
await client.pipelines.stop({
137+
this.runState = "cancelling";
138+
try {
139+
const client = await this.authProvider.getWorkspaceClient();
140+
const update = await client.pipelines.getUpdate({
141+
pipeline_id: this.pipelineId,
142+
update_id: this.runId,
143+
});
144+
// Only stop the pipeline if the tracked update is still running. The stop API stops the
145+
// latest update, which might not be the tracked update.
146+
if (isRunning(update.update?.state)) {
147+
const stopRequest = await client.pipelines.stop({
141148
pipeline_id: this.pipelineId,
142-
})
143-
).wait();
149+
});
150+
await stopRequest.wait();
151+
}
152+
await this.updateRunData(this.runId);
153+
this.runState = "cancelled";
154+
} catch (e) {
155+
this.logger.error("Failed to cancel pipeline run", e);
156+
this.runState = "error";
157+
throw e;
144158
}
145-
const getUpdateResponse = await client.pipelines.getUpdate({
146-
pipeline_id: this.pipelineId,
147-
update_id: this.runId,
148-
});
149-
this.data = getUpdateResponse.update;
150-
this.markCancelled();
151159
}
152160
}

packages/databricks-vscode/src/bundle/run/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ export type RunState =
44
| "unknown"
55
| "error"
66
| "timeout"
7+
| "cancelling"
78
| "cancelled";

packages/databricks-vscode/src/test/e2e/deploy_and_run_pipeline.e2e.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ describe("Deploy and run pipeline", async function () {
8686
resourceExplorerView,
8787
"Pipelines",
8888
pipelineName,
89-
"Completed",
89+
"Success",
9090
// Long timeout, as the pipeline will be waiting for its cluster to start
9191
15 * 60 * 1000
9292
);

packages/databricks-vscode/src/ui/bundle-resource-explorer/JobRunStatusTreeNode.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,14 @@ export class JobRunStatusTreeNode implements BundleResourceExplorerTreeNode {
9292
};
9393
}
9494

95-
const status = JobRunStateUtils.getSimplifiedRunState(this.runDetails);
96-
const icon = RunStateUtils.getThemeIconForStatus(status);
95+
const status =
96+
this.runMonitor?.runState === "cancelling"
97+
? "Cancelling"
98+
: JobRunStateUtils.getSimplifiedRunState(this.runDetails);
99+
97100
return {
98101
label: "Run Status",
99-
iconPath: icon,
102+
iconPath: RunStateUtils.getThemeIconForStatus(status),
100103
description: status,
101104
contextValue: ContextUtils.getContextString({
102105
nodeType: this.type,
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import {ThemeColor, ThemeIcon} from "vscode";
2+
import {ContextUtils} from "./utils";
3+
import {TreeItemTreeNode} from "../TreeItemTreeNode";
4+
import {
5+
EventLevel,
6+
PipelineEvent,
7+
} from "@databricks/databricks-sdk/dist/apis/pipelines";
8+
9+
export class PipelineEventTreeNode<T> extends TreeItemTreeNode<T> {
10+
constructor(
11+
public event: PipelineEvent,
12+
parent: T
13+
) {
14+
super(
15+
{
16+
label: event.message ?? event.event_type ?? "unknown",
17+
iconPath: getEventIcon(event.level),
18+
tooltip: event.message,
19+
contextValue: ContextUtils.getContextString({
20+
nodeType: "pipeline_run_event",
21+
hasPipelineDetails: hasDetails(event),
22+
}),
23+
},
24+
parent
25+
);
26+
}
27+
}
28+
29+
function hasDetails(event: PipelineEvent): boolean {
30+
return (
31+
event.error?.exceptions !== undefined &&
32+
event.error.exceptions.length > 0
33+
);
34+
}
35+
36+
function getEventIcon(level: EventLevel | undefined): ThemeIcon {
37+
switch (level) {
38+
case "ERROR":
39+
return new ThemeIcon(
40+
"error",
41+
new ThemeColor("list.errorForeground")
42+
);
43+
case "INFO":
44+
return new ThemeIcon("info");
45+
case "METRICS":
46+
return new ThemeIcon("dashboard");
47+
case "WARN":
48+
return new ThemeIcon(
49+
"warning",
50+
new ThemeColor("list.warningForeground")
51+
);
52+
default:
53+
return new ThemeIcon("question");
54+
}
55+
}

0 commit comments

Comments
 (0)