Skip to content

Commit

Permalink
Fix "Cluster not found" error when attaching cluster on target change (
Browse files Browse the repository at this point in the history
…#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? -->
  • Loading branch information
kartikgupta-db authored Feb 9, 2024
1 parent 2af50c6 commit ab4e603
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 79 deletions.
10 changes: 3 additions & 7 deletions packages/databricks-vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@
},
{
"command": "databricks.bundle.initNewProject",
"icon": "$(new-folder)",
"title": "Initialize new project",
"enablement": "databricks.context.activated",
"category": "Databricks"
Expand Down Expand Up @@ -336,11 +337,6 @@
],
"menus": {
"view/title": [
{
"command": "databricks.connection.configureLogin",
"when": "view == configurationView",
"group": "navigation@3"
},
{
"command": "databricks.quickstart.open",
"when": "view == configurationView"
Expand Down Expand Up @@ -419,12 +415,12 @@
},
{
"command": "databricks.cluster.stop",
"when": "view == configurationView && viewItem =~ /^databricks.configuration.cluster.(running|pending)$/",
"when": "view == configurationView && viewItem =~ /^databricks.configuration.cluster.*\\.(running|pending).*$/",
"group": "inline@0"
},
{
"command": "databricks.cluster.start",
"when": "view == configurationView && viewItem == databricks.configuration.cluster.terminated",
"when": "view == configurationView && viewItem =~ /databricks.configuration.cluster.*\\.terminated.*/",
"group": "inline@0"
},
{
Expand Down
2 changes: 1 addition & 1 deletion packages/databricks-vscode/src/bundle/BundleWatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class BundleWatcher implements Disposable {
return;
}

await this.bundleFileSet.value.bundleDataCache.invalidate();
this.bundleFileSet.value.bundleDataCache.invalidate();
this._onDidChange.fire();
// to provide additional granularity, we also fire an event when the root bundle file changes
if (this.bundleFileSet.value.isRootBundleFile(e)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {BundleTarget} from "../types";
import lodash from "lodash";
import {workspaceConfigs} from "../../vscode-objs/WorkspaceConfigs";
import {BaseModelWithStateCache} from "../../configuration/models/BaseModelWithStateCache";
import {withOnErrorHandler} from "../../utils/onErrorDecorator";

export type BundleValidateState = {
clusterId?: string;
Expand All @@ -25,9 +26,14 @@ export class BundleValidateModel extends BaseModelWithStateCache<BundleValidateS
) {
super();
this.disposables.push(
this.bundleWatcher.onDidChange(async () => {
await this.stateCache.refresh();
})
this.bundleWatcher.onDidChange(
withOnErrorHandler(
async () => {
await this.stateCache.refresh();
},
{log: true, throw: false}
)
)
);
}

Expand Down
17 changes: 12 additions & 5 deletions packages/databricks-vscode/src/configuration/ConnectionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ export class ConnectionManager implements Disposable {
}

private async loginWithSavedAuth() {
await this.disconnect();
const authProvider = await this.resolveAuth();
if (authProvider === undefined) {
await this.logout();
Expand Down Expand Up @@ -268,18 +269,24 @@ export class ConnectionManager implements Disposable {
this.updateState("CONNECTED");
}

@onError({popup: {prefix: "Can't logout."}})
@Mutex.synchronise("loginLogoutMutex")
async logout() {
private async disconnect() {
this._workspaceClient = undefined;
this._databricksWorkspace = undefined;
await this.updateClusterManager();
await this.updateSyncDestinationMapper();
this.updateState("DISCONNECTED");
}

@onError({popup: {prefix: "Can't logout."}})
async logout() {
await this.disconnect();
if (this.configModel.target !== undefined) {
await this.configModel.set("authProfile", undefined);
await this.configModel.setAuthProvider(undefined);
await Promise.all([
this.configModel.set("authProfile", undefined),
this.configModel.setAuthProvider(undefined),
]);
}
this.updateState("DISCONNECTED");
}

@onError({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export class ConfigModel implements Disposable {
...TOP_LEVEL_VALIDATE_CONFIG_KEYS.map((key) =>
this.bundleValidateModel.onDidChangeKey(key)(async () => {
//refresh cache to trigger onDidChange event
this.configCache.refresh();
await this.configCache.refresh();
})
),
this.bundleRemoteStateModel.onDidChange(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class ConfigurationDataProvider
private readonly configModel: ConfigModel
) {
this.disposables.push(
this.bundleProjectManager.onDidChangeStatus(() => {
this.bundleProjectManager.onDidChangeStatus(async () => {
this._onDidChangeTreeData.fire();
}),
...this.components,
Expand Down
2 changes: 1 addition & 1 deletion packages/databricks-vscode/src/locking/CachedValue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe(__filename, () => {
getterSpy.value = "test2";
expect(getterSpy.callCount).to.equal(1);

await st.invalidate();
st.invalidate();
expect(await st.value).to.equal("test2");
expect(getterSpy.callCount).to.equal(2);
});
Expand Down
27 changes: 15 additions & 12 deletions packages/databricks-vscode/src/locking/CachedValue.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import {EventEmitter, Disposable} from "vscode";
import {Mutex} from ".";
import lodash from "lodash";
import {EventEmitterWithErrorHandler} from "../utils/EventWithErrorHandler";

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

private _value: T | null = null;
private _dirty = true;
private readonly mutex = new Mutex();
private readonly onDidChangeEmitter = new EventEmitter<{
private readonly onDidChangeEmitter = new EventEmitterWithErrorHandler<{
oldValue: T | null;
newValue: T;
}>();
}>({log: true, throw: false});
public readonly onDidChange = this.onDidChangeEmitter.event;

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

get value(): Promise<T> {
if (this._dirty || this._value === null) {
return this.mutex.synchronise(async () => {
return this.mutex.synchronise(async () => {
if (this._dirty || this._value === null) {
const newValue = await this.getter();
if (!lodash.isEqual(newValue, this._value)) {
this.onDidChangeEmitter.fire({
Expand All @@ -65,10 +66,9 @@ export class CachedValue<T> implements Disposable {
this._value = newValue;
this._dirty = false;
return this._value;
});
}

return Promise.resolve(this._value);
}
return this._value;
});
}

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

@Mutex.synchronise("mutex")
async invalidate() {
invalidate() {
this._dirty = true;
}

async refresh() {
await this.invalidate();
await this.value;
this.invalidate();
try {
await this.value;
} finally {
this._dirty = false;
}
}

dispose() {
Expand Down
39 changes: 39 additions & 0 deletions packages/databricks-vscode/src/utils/EventWithErrorHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import {EventEmitter, Disposable} from "vscode";
import {OnErrorProps, withOnErrorHandler} from "./onErrorDecorator";
import lodash from "lodash";

export class EventEmitterWithErrorHandler<T> {
private _eventEmitter = new EventEmitter<T>();
private _event = this._eventEmitter.event;

constructor(private readonly onErrorDefaults: OnErrorProps = {}) {}

get event() {
const fn = (
listener: (e: T) => Promise<unknown>,
opts: {
thisArgs?: any;
disposables?: Disposable[];
onError?: OnErrorProps;
} = {}
) => {
opts = lodash.merge({}, this.onErrorDefaults, opts);

return this._event(
withOnErrorHandler(listener, opts.onError),
opts.thisArgs,
opts.disposables
);
};

return fn.bind(this);
}

fire(event: T) {
this._eventEmitter.fire(event);
}

dispose() {
this._eventEmitter.dispose();
}
}
Loading

0 comments on commit ab4e603

Please sign in to comment.