Skip to content

Commit b5a80a7

Browse files
kartikgupta-dbkartikgupta-db
authored
Refactor configuration UI to better support override and bundle configuration model (#940)
## Changes * Split `ConfigurationDataProvider` into smaller `Component`s. This enables a composable UI where each configuration is represented by a separate config. * Register `ConnectionManager` change listeners during the `init` call only after a login with saved data has been attempted. This race conditions where some changes to the connections manager, such as re-attaching a sync destination due to changes in `ConfigModel` during init, can cause relogin, even before `loginWithSavedAuth` is attempted. **Note**: The UI is a placeholder. We still need to iterate on it to make it both accessible and more inline with our brand design. https://github.com/databricks/databricks-vscode/assets/88345179/354cf62f-28d1-4706-8425-7fa2f00c51c5 ## Tests * manually
1 parent 717c94d commit b5a80a7

16 files changed

+839
-699
lines changed

‎packages/databricks-vscode/package.json‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@
9494
"title": "Start synchronization",
9595
"category": "Databricks",
9696
"enablement": "databricks.context.activated && databricks.context.loggedIn",
97-
"icon": "$(sync)"
97+
"icon": "$(debug-start)"
9898
},
9999
{
100100
"command": "databricks.sync.startFull",
@@ -108,7 +108,7 @@
108108
"title": "Stop synchronization",
109109
"category": "Databricks",
110110
"enablement": "databricks.context.activated && databricks.context.loggedIn",
111-
"icon": "$(sync-ignored)"
111+
"icon": "$(stop-circle)"
112112
},
113113
{
114114
"command": "databricks.cluster.filterByAll",
@@ -184,7 +184,7 @@
184184
{
185185
"command": "databricks.cluster.stop",
186186
"title": "Stop Cluster",
187-
"icon": "$(debug-stop)",
187+
"icon": "$(stop-circle)",
188188
"enablement": "databricks.context.activated && databricks.context.loggedIn",
189189
"category": "Databricks"
190190
},
@@ -223,6 +223,13 @@
223223
"title": "Verify Databricks notebook init scripts",
224224
"enablement": "databricks.context.activated && databricks.context.loggedIn",
225225
"category": "Databricks"
226+
},
227+
{
228+
"command": "databricks.connection.bundle.selectTarget",
229+
"icon": "$(gear)",
230+
"title": "Select a Databricks Asset Bundle target.",
231+
"enablement": "databricks.context.activated",
232+
"category": "Databricks"
226233
}
227234
],
228235
"viewsContainers": {
@@ -323,14 +330,19 @@
323330
"when": "view == clusterView",
324331
"group": "inline@2"
325332
},
333+
{
334+
"command": "databricks.connection.bundle.selectTarget",
335+
"when": "view == configurationView && viewItem =~ /^databricks.configuration.target.*$/",
336+
"group": "inline@2"
337+
},
326338
{
327339
"command": "databricks.connection.configureWorkspace",
328-
"when": "view == configurationView && viewItem == workspace",
340+
"when": "view == configurationView && viewItem =~ /^databricks.configuration.authType.*$/",
329341
"group": "inline@2"
330342
},
331343
{
332344
"command": "databricks.connection.attachClusterQuickPick",
333-
"when": "view == configurationView && viewItem == clusterDetached || view == configurationView && viewItem =~ /^databricks.cluster.*$/",
345+
"when": "view == configurationView && viewItem =~ /^databricks.configuration.cluster.*$/",
334346
"group": "inline@2"
335347
},
336348
{
@@ -340,27 +352,27 @@
340352
},
341353
{
342354
"command": "databricks.connection.attachSyncDestination",
343-
"when": "view == configurationView && viewItem == syncStopped || view == configurationView && viewItem == syncRunning",
355+
"when": "view == configurationView && viewItem =~ /^databricks.configuration.workspaceFsPath.*$/",
344356
"group": "inline@2"
345357
},
346358
{
347359
"command": "databricks.sync.start",
348-
"when": "view == configurationView && viewItem == syncStopped",
360+
"when": "view == configurationView && viewItem == databricks.configuration.workspaceFsPath.stopped",
349361
"group": "inline@0"
350362
},
351363
{
352364
"command": "databricks.sync.stop",
353-
"when": "view == configurationView && viewItem == syncRunning",
365+
"when": "view == configurationView && viewItem =~ /^databricks.configuration.workspaceFsPath.(?!error|stopped|selected).*$/",
354366
"group": "inline@0"
355367
},
356368
{
357369
"command": "databricks.cluster.stop",
358-
"when": "view == configurationView && viewItem =~ /^databricks.cluster.(running|pending)$/",
370+
"when": "view == configurationView && viewItem =~ /^databricks.configuration.cluster.(running|pending)$/",
359371
"group": "inline@0"
360372
},
361373
{
362374
"command": "databricks.cluster.start",
363-
"when": "view == configurationView && viewItem == databricks.cluster.terminated",
375+
"when": "view == configurationView && viewItem == databricks.configuration.cluster.terminated",
364376
"group": "inline@0"
365377
}
366378
],

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ import {Disposable, EventEmitter, Uri, Event} from "vscode";
22
import {
33
BundleConfig,
44
DATABRICKS_CONFIG_KEYS,
5-
DatabricksConfigSource,
65
DatabricksConfig,
76
isBundleConfigKey,
87
isOverrideableConfigKey,
8+
DatabricksConfigSourceMap,
99
} from "./types";
1010
import {ConfigOverrideReaderWriter} from "./ConfigOverrideReaderWriter";
1111
import {BundleConfigReaderWriter} from "./BundleConfigReaderWriter";
@@ -14,6 +14,7 @@ import {BundleWatcher} from "../bundle";
1414
import {CachedValue} from "../locking/CachedValue";
1515
import {StateStorage} from "../vscode-objs/StateStorage";
1616
import _ from "lodash";
17+
import {onError} from "../utils/onErrorDecorator";
1718

1819
function isDirectToBundleConfig(
1920
key: keyof BundleConfig,
@@ -39,7 +40,7 @@ export class ConfigModel implements Disposable {
3940
private readonly configsMutex = new Mutex();
4041
private readonly configCache = new CachedValue<{
4142
config: DatabricksConfig;
42-
source: DatabricksConfigSource;
43+
source: DatabricksConfigSourceMap;
4344
}>(async (oldValue) => {
4445
if (this.target === undefined) {
4546
return {config: {}, source: {}};
@@ -53,7 +54,7 @@ export class ConfigModel implements Disposable {
5354
...overrides,
5455
};
5556

56-
const source: DatabricksConfigSource = {};
57+
const source: DatabricksConfigSourceMap = {};
5758

5859
/* By default undefined values are considered to have come from bundle.
5960
This is because when override for a key is undefined, it means that the key
@@ -121,6 +122,8 @@ export class ConfigModel implements Disposable {
121122
})
122123
);
123124
}
125+
126+
@onError({popup: {prefix: "Failed to initialize configs."}})
124127
public async init() {
125128
await this.readTarget();
126129
}
@@ -146,7 +149,7 @@ export class ConfigModel implements Disposable {
146149
* If not found, try to read from state storage.
147150
* If not found, try to read the default target from bundle.
148151
*/
149-
public async readTarget() {
152+
private async readTarget() {
150153
const targets = Object.keys(
151154
(await this.bundleConfigReaderWriter.targets) ?? {}
152155
);
@@ -202,7 +205,7 @@ export class ConfigModel implements Disposable {
202205
): Promise<
203206
| {
204207
config: DatabricksConfig[T];
205-
source: DatabricksConfigSource[T] | "default";
208+
source: DatabricksConfigSourceMap[T];
206209
}
207210
| undefined
208211
> {

‎packages/databricks-vscode/src/configuration/ConfigurationDataProvider.test.ts‎

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

0 commit comments

Comments
 (0)