Skip to content

Commit 8ce4c9a

Browse files
kartikgupta-dbkartikgupta-db
authored
Refactor config loaders (#978)
## Changes This changes the `*ConfigReaderWriter` classes into `*ConfigModel` classes. We introduce the following architecture: * Configs storage is handled by "Storage Medium" such as `BundleFileSet` and `StateStorage`. They emit event if there is a change. If we can't monitor the underlying medium for changes, we must emit event from the "set" method (see `StateStorage`). * Models are responsible for extracting their respective domain of configs from whatever storage medium they choose. They are also responsible for emitting and event if there is a change in any of the configs in their domain. * Models might need some information such as target and auth params, to automatically refresh the configs in their domain. There are multiple setters which are exposed by each loader for taking this information. We want to have multiple setters and not a single one because multiple pieces of information can come at different times. Eg target -> load auth info and login -> pass auth params to loader * Models are responsible for correctly writing data to the storage medium. ## Tests <!-- How is this tested? -->
1 parent 837d3f2 commit 8ce4c9a

19 files changed

+415
-457
lines changed

‎packages/databricks-vscode/src/cluster/ClusterManager.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import {compute, Time, TimeUnits} from "@databricks/databricks-sdk";
22
import {Cluster} from "../sdk-extensions";
33
import {CancellationTokenSource, Disposable} from "vscode";
4-
import * as lodash from "lodash";
4+
import lodash from "lodash";
5+
import {logging, context, Context} from "@databricks/databricks-sdk";
6+
import {Loggers} from "../logger";
57
export class ClusterManager implements Disposable {
68
private cancellationTokenSource?: CancellationTokenSource;
79
private refreshTimer?: NodeJS.Timeout;
@@ -14,10 +16,16 @@ export class ClusterManager implements Disposable {
1416
this.setInterval();
1517
}
1618

17-
private setInterval() {
19+
@logging.withLogContext(Loggers.Extension)
20+
private setInterval(@context ctx?: Context) {
1821
this.refreshTimer = setInterval(async () => {
1922
const oldState = this.cluster.state;
20-
await this.cluster.refresh();
23+
try {
24+
await this.cluster.refresh();
25+
} catch (e: any) {
26+
ctx?.logger?.error("Error refreshing cluster", e);
27+
}
28+
2129
if (!lodash.isEqual(oldState, this.cluster.state)) {
2230
this.onChange(this.cluster.state);
2331
}

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

Lines changed: 0 additions & 213 deletions
This file was deleted.

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

Lines changed: 0 additions & 58 deletions
This file was deleted.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {FileUtils, UrlUtils} from "../utils";
1616
import {workspaceConfigs} from "../vscode-objs/WorkspaceConfigs";
1717
import {WorkspaceFsCommands} from "../workspace-fs";
1818
import path from "node:path";
19-
import {ConfigModel} from "./ConfigModel";
19+
import {ConfigModel} from "./models/ConfigModel";
2020

2121
function formatQuickPickClusterSize(sizeInMB: number): string {
2222
if (sizeInMB > 1024) {
@@ -200,7 +200,7 @@ export class ConnectionCommands implements Disposable {
200200
}
201201

202202
async selectTarget() {
203-
const targets = await this.configModel.bundleConfigReaderWriter.targets;
203+
const targets = await this.configModel.bundleFileConfigModel.targets;
204204
const currentTarget = this.configModel.target;
205205
if (targets === undefined) {
206206
return;

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {ClusterManager} from "../cluster/ClusterManager";
1212
import {DatabricksWorkspace} from "./DatabricksWorkspace";
1313
import {CustomWhenContext} from "../vscode-objs/CustomWhenContext";
1414
import {workspaceConfigs} from "../vscode-objs/WorkspaceConfigs";
15-
import {ConfigModel} from "./ConfigModel";
15+
import {ConfigModel} from "./models/ConfigModel";
1616
import {onError} from "../utils/onErrorDecorator";
1717
import {AuthProvider} from "./auth/AuthProvider";
1818
import {Mutex} from "../locking";
@@ -116,22 +116,19 @@ export class ConnectionManager implements Disposable {
116116
await this.loginWithSavedAuth();
117117

118118
this.disposables.push(
119-
this.configModel.onDidChange(
120-
"workspaceFsPath",
119+
this.configModel.onDidChange("workspaceFsPath")(
121120
this.updateSyncDestinationMapper,
122121
this
123122
),
124-
this.configModel.onDidChange(
125-
"clusterId",
123+
this.configModel.onDidChange("clusterId")(
126124
this.updateClusterManager,
127125
this
128126
),
129-
this.configModel.onDidChange(
130-
"target",
127+
this.configModel.onDidChange("target")(
131128
this.loginWithSavedAuth,
132129
this
133130
),
134-
this.configModel.onDidChange("authParams", async () => {
131+
this.configModel.onDidChange("authParams")(async () => {
135132
const config = await this.configModel.getS("authParams");
136133
if (config === undefined) {
137134
return;
@@ -271,14 +268,16 @@ export class ConnectionManager implements Disposable {
271268
}
272269
}
273270

274-
@onError({popup: {prefix: "Can't logout. "}})
271+
@onError({popup: {prefix: "Can't logout"}})
275272
@Mutex.synchronise("loginLogoutMutex")
276273
async logout() {
277274
this._workspaceClient = undefined;
278275
this._databricksWorkspace = undefined;
279276
await this.updateClusterManager();
280277
await this.updateSyncDestinationMapper();
281-
await this.configModel.set("authParams", undefined);
278+
if (this.configModel.target !== undefined) {
279+
await this.configModel.set("authParams", undefined);
280+
}
282281
this.updateState("DISCONNECTED");
283282
}
284283

0 commit comments

Comments
 (0)