Skip to content

Commit 65f03ee

Browse files
kartikgupta-dbkartikgupta-db
authored
Redirect summarise and validate errors to bundle output panel (#1076)
## Changes <img width="1179" alt="image" src="https://github.com/databricks/databricks-vscode/assets/88345179/3bdccb54-f433-4dba-bcc6-bbbb409c82ff"> https://github.com/databricks/databricks-vscode/assets/88345179/ce30253f-bb4c-4377-b1f5-867c12f28dcc <img width="549" alt="image" src="https://github.com/databricks/databricks-vscode/assets/88345179/858241ac-f744-427b-81eb-c79bb85b0c65"> ## Tests <!-- How is this tested? -->
1 parent 7412d95 commit 65f03ee

File tree

12 files changed

+388
-242
lines changed

12 files changed

+388
-242
lines changed

‎packages/databricks-vscode/package.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,12 @@
268268
"title": "Initialize new project",
269269
"enablement": "databricks.context.activated",
270270
"category": "Databricks"
271+
},
272+
{
273+
"command": "databricks.bundle.showLogs",
274+
"title": "Show bundle logs",
275+
"enablement": "databricks.context.activated",
276+
"category": "Databricks"
271277
}
272278
],
273279
"viewsContainers": {

‎packages/databricks-vscode/src/bundle/models/BundleRemoteStateModel.ts‎

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@ import lodash from "lodash";
99
import {WorkspaceConfigs} from "../../vscode-objs/WorkspaceConfigs";
1010
import {logging} from "@databricks/databricks-sdk";
1111
import {Loggers} from "../../logger";
12-
import {Context, context} from "@databricks/databricks-sdk";
13-
import {BundleValidateModel} from "./BundleValidateModel";
14-
import {withOnErrorHandler} from "../../utils/onErrorDecorator";
1512

1613
/* eslint-disable @typescript-eslint/naming-convention */
1714
export type BundleResourceModifiedStatus = "created" | "deleted" | "updated";
@@ -47,33 +44,23 @@ export class BundleRemoteStateModel extends BaseModelWithStateCache<BundleRemote
4744
public authProvider: AuthProvider | undefined;
4845
protected mutex = new Mutex();
4946
private refreshInterval: NodeJS.Timeout | undefined;
47+
private logger = logging.NamedLogger.getOrCreate(Loggers.Bundle);
5048

5149
constructor(
5250
private readonly cli: CliWrapper,
5351
private readonly workspaceFolder: Uri,
54-
private readonly workspaceConfigs: WorkspaceConfigs,
55-
private readonly bundleValidateModel: BundleValidateModel
52+
private readonly workspaceConfigs: WorkspaceConfigs
5653
) {
5754
super();
58-
this.bundleValidateModel.onDidChange(
59-
withOnErrorHandler(
60-
async () => {
61-
this.refresh();
62-
},
63-
{log: true, throw: false}
64-
)
65-
);
6655
}
6756

57+
@Mutex.synchronise("mutex")
6858
public async refresh() {
6959
return await this.stateCache.refresh();
7060
}
7161

7262
@Mutex.synchronise("mutex")
73-
public async deploy(
74-
onStdOut?: (data: string) => void,
75-
onStdErr?: (data: string) => void
76-
) {
63+
public async deploy() {
7764
if (this.target === undefined) {
7865
throw new Error("Target is undefined");
7966
}
@@ -86,8 +73,7 @@ export class BundleRemoteStateModel extends BaseModelWithStateCache<BundleRemote
8673
this.authProvider,
8774
this.workspaceFolder,
8875
this.workspaceConfigs.databrickscfgLocation,
89-
onStdOut,
90-
onStdErr
76+
this.logger
9177
);
9278
}
9379

@@ -108,17 +94,6 @@ export class BundleRemoteStateModel extends BaseModelWithStateCache<BundleRemote
10894
);
10995
}
11096

111-
@logging.withLogContext(Loggers.Extension)
112-
public init(@context ctx?: Context) {
113-
this.refreshInterval = setInterval(async () => {
114-
try {
115-
await this.stateCache.refresh();
116-
} catch (e) {
117-
ctx?.logger?.error("Unable to refresh bundle remote state", e);
118-
}
119-
}, this.workspaceConfigs.bundleRemoteStateRefreshInterval);
120-
}
121-
12297
@Mutex.synchronise("mutex")
12398
public async setTarget(target: string | undefined) {
12499
if (this.target === target) {
@@ -148,7 +123,8 @@ export class BundleRemoteStateModel extends BaseModelWithStateCache<BundleRemote
148123
this.target,
149124
this.authProvider,
150125
this.workspaceFolder,
151-
this.workspaceConfigs.databrickscfgLocation
126+
this.workspaceConfigs.databrickscfgLocation,
127+
this.logger
152128
);
153129

154130
if (output === "" || output === undefined) {

‎packages/databricks-vscode/src/bundle/models/BundleValidateModel.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import lodash from "lodash";
88
import {workspaceConfigs} from "../../vscode-objs/WorkspaceConfigs";
99
import {BaseModelWithStateCache} from "../../configuration/models/BaseModelWithStateCache";
1010
import {withOnErrorHandler} from "../../utils/onErrorDecorator";
11+
import {logging} from "@databricks/databricks-sdk";
12+
import {Loggers} from "../../logger";
1113

1214
export type BundleValidateState = {
1315
clusterId?: string;
@@ -18,6 +20,7 @@ export class BundleValidateModel extends BaseModelWithStateCache<BundleValidateS
1820
private target: string | undefined;
1921
private authProvider: AuthProvider | undefined;
2022
protected mutex = new Mutex();
23+
protected logger = logging.NamedLogger.getOrCreate(Loggers.Bundle);
2124

2225
constructor(
2326
private readonly bundleWatcher: BundleWatcher,
@@ -67,7 +70,8 @@ export class BundleValidateModel extends BaseModelWithStateCache<BundleValidateS
6770
this.target,
6871
this.authProvider,
6972
this.workspaceFolder,
70-
workspaceConfigs.databrickscfgLocation
73+
workspaceConfigs.databrickscfgLocation,
74+
this.logger
7175
)
7276
) as BundleTarget;
7377

‎packages/databricks-vscode/src/cli/CliWrapper.ts‎

Lines changed: 101 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ async function waitForProcess(
7474
reject(new ProcessError(stderr.join("\n"), code));
7575
}
7676
});
77-
p.on("error", reject);
77+
p.on("error", (e) => new ProcessError(e.message, null));
7878
});
7979

8080
return output.join("");
@@ -247,12 +247,19 @@ export class CliWrapper {
247247
target: string,
248248
authProvider: AuthProvider,
249249
workspaceFolder: Uri,
250-
configfilePath?: string
250+
configfilePath?: string,
251+
logger?: logging.NamedLogger
251252
) {
252-
const {stdout} = await execFile(
253-
this.cliPath,
254-
["bundle", "validate", "--target", target],
255-
{
253+
const cmd = [this.cliPath, "bundle", "validate", "--target", target];
254+
255+
logger?.info(
256+
`Reading local bundle configuration for target ${target}...`,
257+
{bundleOpName: "validate"}
258+
);
259+
logger?.debug(quote(cmd));
260+
261+
try {
262+
const {stdout, stderr} = await execFile(cmd[0], cmd.slice(1), {
256263
cwd: workspaceFolder.fsPath,
257264
env: {
258265
...EnvVarGenerators.getEnvVarsForCli(configfilePath),
@@ -263,22 +270,43 @@ export class CliWrapper {
263270
DATABRICKS_CLUSTER_ID: this.clusterId,
264271
},
265272
shell: true,
266-
}
267-
);
273+
});
274+
const output = stdout + stderr;
268275

269-
return stdout;
276+
logger?.info("Finished reading local bundle configuration.", {
277+
bundleOpName: "validate",
278+
});
279+
logger?.debug(output);
280+
return output;
281+
} catch (e: any) {
282+
logger?.error(
283+
`Failed to read local bundle configuration. ${e.message ?? ""}`,
284+
{
285+
...e,
286+
bundleOpName: "validate",
287+
}
288+
);
289+
throw new ProcessError(e.message, e.code);
290+
}
270291
}
271292

272293
async bundleSummarise(
273294
target: string,
274295
authProvider: AuthProvider,
275296
workspaceFolder: Uri,
276-
configfilePath?: string
297+
configfilePath?: string,
298+
logger?: logging.NamedLogger
277299
) {
278-
const {stdout, stderr} = await execFile(
279-
this.cliPath,
280-
["bundle", "summary", "--target", target],
281-
{
300+
const cmd = [this.cliPath, "bundle", "summary", "--target", target];
301+
302+
logger?.info(
303+
`Refreshing bundle configuration for target ${target}...`,
304+
{bundleOpName: "summarize"}
305+
);
306+
logger?.debug(quote(cmd));
307+
308+
try {
309+
const {stdout, stderr} = await execFile(cmd[0], cmd.slice(1), {
282310
cwd: workspaceFolder.fsPath,
283311
env: {
284312
...EnvVarGenerators.getEnvVarsForCli(configfilePath),
@@ -289,13 +317,24 @@ export class CliWrapper {
289317
DATABRICKS_CLUSTER_ID: this.clusterId,
290318
},
291319
shell: true,
292-
}
293-
);
320+
});
294321

295-
if (stderr !== "") {
296-
throw new Error(stderr);
322+
const output = stdout + stderr;
323+
logger?.info("Bundle configuration refreshed.", {
324+
bundleOpName: "summarize",
325+
});
326+
logger?.debug(output);
327+
return output;
328+
} catch (e: any) {
329+
logger?.error(
330+
`Failed to refresh bundle configuration. ${e.message ?? ""}`,
331+
{
332+
...e,
333+
bundleOpName: "summarize",
334+
}
335+
);
336+
throw new ProcessError(e.message, e.code);
297337
}
298-
return stdout;
299338
}
300339

301340
getBundleInitEnvVars(authProvider: AuthProvider) {
@@ -337,17 +376,22 @@ export class CliWrapper {
337376
authProvider: AuthProvider,
338377
workspaceFolder: Uri,
339378
configfilePath?: string,
340-
onStdOut?: (data: string) => void,
341-
onStdError?: (data: string) => void
379+
logger?: logging.NamedLogger
342380
) {
343381
const cmd = [this.cliPath, "bundle", "deploy", "--target", target];
344-
if (onStdError) {
345-
onStdError(`Deploying the bundle for target ${target}...\n`);
346-
if (this.clusterId) {
347-
onStdError(`DATABRICKS_CLUSTER_ID=${this.clusterId}\n\n`);
348-
}
349-
onStdOut?.(quote(cmd) + "\n\n");
382+
383+
logger?.info(`Deploying the bundle for target ${target}...`, {
384+
bundleOpName: "deploy",
385+
});
386+
if (this.clusterId) {
387+
logger?.info(`DATABRICKS_CLUSTER_ID=${this.clusterId}`, {
388+
bundleOpName: "deploy",
389+
});
350390
}
391+
logger?.info(quote(cmd), {
392+
bundleOpName: "deploy",
393+
});
394+
351395
const p = spawn(cmd[0], cmd.slice(1), {
352396
cwd: workspaceFolder.fsPath,
353397
env: {
@@ -361,7 +405,36 @@ export class CliWrapper {
361405
shell: true,
362406
});
363407

364-
return await waitForProcess(p, onStdOut, onStdError);
408+
try {
409+
const output = await waitForProcess(
410+
p,
411+
(message) => {
412+
logger?.info(message, {
413+
outputStream: "stdout",
414+
bundleOpName: "deploy",
415+
});
416+
},
417+
(message) => {
418+
logger?.info(message, {
419+
outputStream: "stderr",
420+
bundleOpName: "deploy",
421+
});
422+
}
423+
);
424+
logger?.info("Bundle deployed successfully", {
425+
bundleOpName: "deploy",
426+
});
427+
logger?.debug(output, {
428+
bundleOpName: "deploy",
429+
});
430+
return output;
431+
} catch (e: any) {
432+
logger?.error("Failed to deploy the bundle", {
433+
...e,
434+
bundleOpName: "deploy",
435+
});
436+
throw e;
437+
}
365438
}
366439

367440
getBundleRunCommand(

‎packages/databricks-vscode/src/configuration/ConnectionManager.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {WorkspaceClient, ApiClient, logging} from "@databricks/databricks-sdk";
22
import {Cluster} from "../sdk-extensions";
3-
import {EventEmitter, Uri, window, Disposable} from "vscode";
4-
import {CliWrapper} from "../cli/CliWrapper";
3+
import {EventEmitter, Uri, window, Disposable, commands} from "vscode";
4+
import {CliWrapper, ProcessError} from "../cli/CliWrapper";
55
import {
66
SyncDestinationMapper,
77
RemoteUri,
@@ -250,8 +250,21 @@ export class ConnectionManager implements Disposable {
250250
`Can't connect to the workspace`,
251251
e
252252
);
253-
if (e instanceof Error) {
254-
window.showWarningMessage(
253+
if (e instanceof ProcessError) {
254+
window
255+
.showErrorMessage(
256+
`Can't connect to the workspace. Error executing Databricks CLI command.`,
257+
"Show Logs"
258+
)
259+
.then((choice) => {
260+
if (choice === "Show Logs") {
261+
commands.executeCommand(
262+
"databricks.bundle.showLogs"
263+
);
264+
}
265+
});
266+
} else if (e instanceof Error) {
267+
window.showErrorMessage(
255268
`Can't connect to the workspace: "${e.message}"."`
256269
);
257270
}

‎packages/databricks-vscode/src/configuration/models/ConfigModel.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,6 @@ export class ConfigModel implements Disposable {
149149
@onError({popup: true})
150150
public async init() {
151151
await this.readTarget();
152-
this.bundleRemoteStateModel.init();
153152
}
154153

155154
get targets() {

‎packages/databricks-vscode/src/extension.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,11 @@ export async function activate(
164164
"databricks.logs.openFolder",
165165
loggerManager.openLogFolder,
166166
loggerManager
167+
),
168+
telemetry.registerCommand(
169+
"databricks.bundle.showLogs",
170+
() => loggerManager.showOutputChannel("Databricks Bundle Logs"),
171+
loggerManager
167172
)
168173
);
169174

@@ -195,8 +200,7 @@ export async function activate(
195200
const bundleRemoteStateModel = new BundleRemoteStateModel(
196201
cli,
197202
workspaceUri,
198-
workspaceConfigs,
199-
bundleValidateModel
203+
workspaceConfigs
200204
);
201205
const configModel = new ConfigModel(
202206
bundleValidateModel,
@@ -567,7 +571,7 @@ export async function activate(
567571
),
568572
telemetry.registerCommand(
569573
"databricks.bundle.refreshRemoteState",
570-
bundleCommands.refreshRemoteStateCommand,
574+
bundleCommands.refreshCommand,
571575
bundleCommands
572576
),
573577
telemetry.registerCommand(

0 commit comments

Comments
 (0)