Skip to content

Commit ab4e603

Browse files
Fix "Cluster not found" error when attaching cluster on target change (#1003)
## Changes * Add onError handler to be able to log errors from lambdas. Use it to catch errors in bundleWatcher.onDidChange callback in bundleValidate. * Remove configure workspace command from the configuration view header. Expose init new project command. * Call disconnect before logging in with saved auth. This allows us to clear any existing auth information (workspace client) * Bring back cluster start stop icons ## Tests <!-- How is this tested? -->
1 parent 2af50c6 commit ab4e603

File tree

10 files changed

+165
-79
lines changed

10 files changed

+165
-79
lines changed

packages/databricks-vscode/package.json

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,7 @@
264264
},
265265
{
266266
"command": "databricks.bundle.initNewProject",
267+
"icon": "$(new-folder)",
267268
"title": "Initialize new project",
268269
"enablement": "databricks.context.activated",
269270
"category": "Databricks"
@@ -336,11 +337,6 @@
336337
],
337338
"menus": {
338339
"view/title": [
339-
{
340-
"command": "databricks.connection.configureLogin",
341-
"when": "view == configurationView",
342-
"group": "navigation@3"
343-
},
344340
{
345341
"command": "databricks.quickstart.open",
346342
"when": "view == configurationView"
@@ -419,12 +415,12 @@
419415
},
420416
{
421417
"command": "databricks.cluster.stop",
422-
"when": "view == configurationView && viewItem =~ /^databricks.configuration.cluster.(running|pending)$/",
418+
"when": "view == configurationView && viewItem =~ /^databricks.configuration.cluster.*\\.(running|pending).*$/",
423419
"group": "inline@0"
424420
},
425421
{
426422
"command": "databricks.cluster.start",
427-
"when": "view == configurationView && viewItem == databricks.configuration.cluster.terminated",
423+
"when": "view == configurationView && viewItem =~ /databricks.configuration.cluster.*\\.terminated.*/",
428424
"group": "inline@0"
429425
},
430426
{

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export class BundleWatcher implements Disposable {
4949
return;
5050
}
5151

52-
await this.bundleFileSet.value.bundleDataCache.invalidate();
52+
this.bundleFileSet.value.bundleDataCache.invalidate();
5353
this._onDidChange.fire();
5454
// to provide additional granularity, we also fire an event when the root bundle file changes
5555
if (this.bundleFileSet.value.isRootBundleFile(e)) {

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {BundleTarget} from "../types";
77
import lodash from "lodash";
88
import {workspaceConfigs} from "../../vscode-objs/WorkspaceConfigs";
99
import {BaseModelWithStateCache} from "../../configuration/models/BaseModelWithStateCache";
10+
import {withOnErrorHandler} from "../../utils/onErrorDecorator";
1011

1112
export type BundleValidateState = {
1213
clusterId?: string;
@@ -25,9 +26,14 @@ export class BundleValidateModel extends BaseModelWithStateCache<BundleValidateS
2526
) {
2627
super();
2728
this.disposables.push(
28-
this.bundleWatcher.onDidChange(async () => {
29-
await this.stateCache.refresh();
30-
})
29+
this.bundleWatcher.onDidChange(
30+
withOnErrorHandler(
31+
async () => {
32+
await this.stateCache.refresh();
33+
},
34+
{log: true, throw: false}
35+
)
36+
)
3137
);
3238
}
3339

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ export class ConnectionManager implements Disposable {
175175
}
176176

177177
private async loginWithSavedAuth() {
178+
await this.disconnect();
178179
const authProvider = await this.resolveAuth();
179180
if (authProvider === undefined) {
180181
await this.logout();
@@ -268,18 +269,24 @@ export class ConnectionManager implements Disposable {
268269
this.updateState("CONNECTED");
269270
}
270271

271-
@onError({popup: {prefix: "Can't logout."}})
272272
@Mutex.synchronise("loginLogoutMutex")
273-
async logout() {
273+
private async disconnect() {
274274
this._workspaceClient = undefined;
275275
this._databricksWorkspace = undefined;
276276
await this.updateClusterManager();
277277
await this.updateSyncDestinationMapper();
278+
this.updateState("DISCONNECTED");
279+
}
280+
281+
@onError({popup: {prefix: "Can't logout."}})
282+
async logout() {
283+
await this.disconnect();
278284
if (this.configModel.target !== undefined) {
279-
await this.configModel.set("authProfile", undefined);
280-
await this.configModel.setAuthProvider(undefined);
285+
await Promise.all([
286+
this.configModel.set("authProfile", undefined),
287+
this.configModel.setAuthProvider(undefined),
288+
]);
281289
}
282-
this.updateState("DISCONNECTED");
283290
}
284291

285292
@onError({

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ export class ConfigModel implements Disposable {
137137
...TOP_LEVEL_VALIDATE_CONFIG_KEYS.map((key) =>
138138
this.bundleValidateModel.onDidChangeKey(key)(async () => {
139139
//refresh cache to trigger onDidChange event
140-
this.configCache.refresh();
140+
await this.configCache.refresh();
141141
})
142142
),
143143
this.bundleRemoteStateModel.onDidChange(async () => {

packages/databricks-vscode/src/configuration/ui/ConfigurationDataProvider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export class ConfigurationDataProvider
4242
private readonly configModel: ConfigModel
4343
) {
4444
this.disposables.push(
45-
this.bundleProjectManager.onDidChangeStatus(() => {
45+
this.bundleProjectManager.onDidChangeStatus(async () => {
4646
this._onDidChangeTreeData.fire();
4747
}),
4848
...this.components,

packages/databricks-vscode/src/locking/CachedValue.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe(__filename, () => {
3535
getterSpy.value = "test2";
3636
expect(getterSpy.callCount).to.equal(1);
3737

38-
await st.invalidate();
38+
st.invalidate();
3939
expect(await st.value).to.equal("test2");
4040
expect(getterSpy.callCount).to.equal(2);
4141
});

packages/databricks-vscode/src/locking/CachedValue.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
import {EventEmitter, Disposable} from "vscode";
22
import {Mutex} from ".";
33
import lodash from "lodash";
4+
import {EventEmitterWithErrorHandler} from "../utils/EventWithErrorHandler";
45

56
export class CachedValue<T> implements Disposable {
67
private disposables: Disposable[] = [];
78

89
private _value: T | null = null;
910
private _dirty = true;
1011
private readonly mutex = new Mutex();
11-
private readonly onDidChangeEmitter = new EventEmitter<{
12+
private readonly onDidChangeEmitter = new EventEmitterWithErrorHandler<{
1213
oldValue: T | null;
1314
newValue: T;
14-
}>();
15+
}>({log: true, throw: false});
1516
public readonly onDidChange = this.onDidChangeEmitter.event;
1617

1718
constructor(private readonly getter: () => Promise<T>) {
@@ -53,8 +54,8 @@ export class CachedValue<T> implements Disposable {
5354
}
5455

5556
get value(): Promise<T> {
56-
if (this._dirty || this._value === null) {
57-
return this.mutex.synchronise(async () => {
57+
return this.mutex.synchronise(async () => {
58+
if (this._dirty || this._value === null) {
5859
const newValue = await this.getter();
5960
if (!lodash.isEqual(newValue, this._value)) {
6061
this.onDidChangeEmitter.fire({
@@ -65,10 +66,9 @@ export class CachedValue<T> implements Disposable {
6566
this._value = newValue;
6667
this._dirty = false;
6768
return this._value;
68-
});
69-
}
70-
71-
return Promise.resolve(this._value);
69+
}
70+
return this._value;
71+
});
7272
}
7373

7474
private readonly onDidChangeKeyEmitters = new Map<
@@ -83,14 +83,17 @@ export class CachedValue<T> implements Disposable {
8383
return this.onDidChangeKeyEmitters.get(key)!.event;
8484
}
8585

86-
@Mutex.synchronise("mutex")
87-
async invalidate() {
86+
invalidate() {
8887
this._dirty = true;
8988
}
9089

9190
async refresh() {
92-
await this.invalidate();
93-
await this.value;
91+
this.invalidate();
92+
try {
93+
await this.value;
94+
} finally {
95+
this._dirty = false;
96+
}
9497
}
9598

9699
dispose() {
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import {EventEmitter, Disposable} from "vscode";
2+
import {OnErrorProps, withOnErrorHandler} from "./onErrorDecorator";
3+
import lodash from "lodash";
4+
5+
export class EventEmitterWithErrorHandler<T> {
6+
private _eventEmitter = new EventEmitter<T>();
7+
private _event = this._eventEmitter.event;
8+
9+
constructor(private readonly onErrorDefaults: OnErrorProps = {}) {}
10+
11+
get event() {
12+
const fn = (
13+
listener: (e: T) => Promise<unknown>,
14+
opts: {
15+
thisArgs?: any;
16+
disposables?: Disposable[];
17+
onError?: OnErrorProps;
18+
} = {}
19+
) => {
20+
opts = lodash.merge({}, this.onErrorDefaults, opts);
21+
22+
return this._event(
23+
withOnErrorHandler(listener, opts.onError),
24+
opts.thisArgs,
25+
opts.disposables
26+
);
27+
};
28+
29+
return fn.bind(this);
30+
}
31+
32+
fire(event: T) {
33+
this._eventEmitter.fire(event);
34+
}
35+
36+
dispose() {
37+
this._eventEmitter.dispose();
38+
}
39+
}

0 commit comments

Comments
 (0)