Skip to content

Commit ceb6324

Browse files
Remove API timeout limit for execution context runs (#487)
Fixes #482 --------- Co-authored-by: Fabian Jakobs <fabian.jakobs@databricks.com>
1 parent 1f67001 commit ceb6324

File tree

5 files changed

+38
-11
lines changed

5 files changed

+38
-11
lines changed

packages/databricks-sdk-js/src/services/Command.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import {EventEmitter} from "events";
2-
import retry, {RetriableError} from "../retries/retries";
2+
import retry, {DEFAULT_MAX_TIMEOUT, RetriableError} from "../retries/retries";
33
import {ExecutionContext} from "./ExecutionContext";
44
import {CancellationToken} from "../types";
55
import {CommandExecutionService, CommandStatusResponse} from "../apis/commands";
6+
import Time from "../retries/Time";
67

78
interface CommandErrorParams {
89
commandId: string;
@@ -109,9 +110,11 @@ export class Command extends EventEmitter {
109110
}
110111

111112
async response(
112-
cancellationToken?: CancellationToken
113+
cancellationToken?: CancellationToken,
114+
timeout: Time = DEFAULT_MAX_TIMEOUT
113115
): Promise<CommandStatusResponse> {
114116
await retry({
117+
timeout: timeout,
115118
fn: async () => {
116119
await this.refresh();
117120

@@ -143,7 +146,8 @@ export class Command extends EventEmitter {
143146
context: ExecutionContext,
144147
command: string,
145148
onStatusUpdate: StatusUpdateListener = () => {},
146-
cancellationToken?: CancellationToken
149+
cancellationToken?: CancellationToken,
150+
timeout: Time = DEFAULT_MAX_TIMEOUT
147151
): Promise<CommandWithResult> {
148152
const cmd = new Command(context);
149153

@@ -158,7 +162,7 @@ export class Command extends EventEmitter {
158162

159163
cmd.id = executeApiResponse.id;
160164

161-
const executionResult = await cmd.response(cancellationToken);
165+
const executionResult = await cmd.response(cancellationToken, timeout);
162166

163167
return {cmd: cmd, result: executionResult};
164168
}

packages/databricks-sdk-js/src/services/ExecutionContext.integ.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import assert from "assert";
66
import {IntegrationTestSetup} from "../test/IntegrationTestSetup";
77
import {mock, when, instance} from "ts-mockito";
88
import {TokenFixture} from "../test/fixtures/TokenFixtures";
9+
import {DEFAULT_MAX_TIMEOUT} from "../retries/retries";
910

1011
describe(__filename, function () {
1112
let integSetup: IntegrationTestSetup;
@@ -56,7 +57,8 @@ describe(__filename, function () {
5657
const {cmd, result} = await context.execute(
5758
"while True: pass",
5859
undefined,
59-
instance(token)
60+
instance(token),
61+
DEFAULT_MAX_TIMEOUT
6062
);
6163
// The API surfaces an exception when a command is cancelled
6264
// The cancellation itself proceeds as expected, but the status

packages/databricks-sdk-js/src/services/ExecutionContext.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import {CancellationToken} from "..";
1+
import {CancellationToken, Time} from "..";
22
import {ApiClient} from "../api-client";
33
import {CommandExecutionService, Language} from "../apis/commands";
4+
import {DEFAULT_MAX_TIMEOUT} from "../retries/retries";
45
import {Cluster} from "./Cluster";
56
import {Command, CommandWithResult, StatusUpdateListener} from "./Command";
67

@@ -34,9 +35,16 @@ export class ExecutionContext {
3435
async execute(
3536
command: string,
3637
onStatusUpdate: StatusUpdateListener = () => {},
37-
token?: CancellationToken
38+
token?: CancellationToken,
39+
timeout: Time = DEFAULT_MAX_TIMEOUT
3840
): Promise<CommandWithResult> {
39-
return await Command.execute(this, command, onStatusUpdate, token);
41+
return await Command.execute(
42+
this,
43+
command,
44+
onStatusUpdate,
45+
token,
46+
timeout
47+
);
4048
}
4149

4250
async destroy() {

packages/databricks-vscode/src/run/DatabricksRuntime.test.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@ describe(__filename, () => {
2828

2929
executionContextMock = mock(ExecutionContext);
3030
when(
31-
executionContextMock.execute(anything(), anything(), anything())
31+
executionContextMock.execute(
32+
anything(),
33+
anything(),
34+
anything(),
35+
anything()
36+
)
3237
).thenResolve({
3338
cmd: mock(Command),
3439
result: {
@@ -122,7 +127,12 @@ describe(__filename, () => {
122127

123128
it("should handle failed executions", async () => {
124129
when(
125-
executionContextMock.execute(anything(), anything(), anything())
130+
executionContextMock.execute(
131+
anything(),
132+
anything(),
133+
anything(),
134+
anything()
135+
)
126136
).thenResolve({
127137
cmd: mock(Command),
128138
result: {

packages/databricks-vscode/src/run/DatabricksRuntime.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {CodeSynchronizer} from "../sync/CodeSynchronizer";
2525
import * as fs from "node:fs/promises";
2626
import {parseErrorResult} from "./ErrorParser";
2727
import path from "node:path";
28+
import {Time, TimeUnits} from "@databricks/databricks-sdk";
2829

2930
export interface OutputEvent {
3031
type: "prio" | "out" | "err";
@@ -151,6 +152,7 @@ export class DatabricksRuntime implements Disposable {
151152
// We wait for sync to complete so that the local files are consistant
152153
// with the remote repo files
153154
await this.codeSynchronizer.waitForSyncComplete();
155+
await commands.executeCommand("workbench.panel.repl.view.focus");
154156

155157
log(
156158
`Running ${syncDestination.localUri.relativePath(
@@ -166,7 +168,8 @@ export class DatabricksRuntime implements Disposable {
166168
envVars
167169
),
168170
undefined,
169-
this.token
171+
this.token,
172+
new Time(240, TimeUnits.hours)
170173
);
171174
const result = response.result;
172175

0 commit comments

Comments
 (0)