Skip to content

Commit 837d3f2

Browse files
kartikgupta-dbkartikgupta-db
authored
Dissallow all overrides except cluster ID for dev mode and Auth Params for all modes (#972)
## Changes * Keeping in line with the intended CUJs we only want to allow overriding cluster id and that too only for dev mode. * This also hides cluster ID in other modes since it is irrelevant there. ## Tests <!-- How is this tested? -->
1 parent 1835fc2 commit 837d3f2

File tree

10 files changed

+71
-297
lines changed

10 files changed

+71
-297
lines changed

‎packages/databricks-vscode/package.json‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -317,12 +317,7 @@
317317
"view/item/context": [
318318
{
319319
"command": "databricks.utils.openExternal",
320-
"when": "view == configurationView && viewItem == workspace || view == configurationView && viewItem =~ /^.*cluster.*$/ || view == configurationView && viewItem =~ /^sync.*$/",
321-
"group": "inline@1"
322-
},
323-
{
324-
"command": "databricks.utils.openExternal",
325-
"when": "view == clusterView && viewItem == cluster",
320+
"when": "view == configurationView && viewItem =~ /^databricks.configuration.(cluster|sync).*$/",
326321
"group": "inline@1"
327322
},
328323
{
@@ -345,24 +340,14 @@
345340
"when": "view == configurationView && viewItem =~ /^databricks.configuration.cluster.*$/",
346341
"group": "inline@2"
347342
},
348-
{
349-
"command": "databricks.connection.attachSyncDestination",
350-
"when": "view == configurationView && viewItem == syncDetached",
351-
"group": "inline@2"
352-
},
353-
{
354-
"command": "databricks.connection.attachSyncDestination",
355-
"when": "view == configurationView && viewItem =~ /^databricks.configuration.workspaceFsPath.*$/",
356-
"group": "inline@2"
357-
},
358343
{
359344
"command": "databricks.sync.start",
360-
"when": "view == configurationView && viewItem == databricks.configuration.workspaceFsPath.stopped",
345+
"when": "view == configurationView && viewItem == databricks.configuration.sync.stopped",
361346
"group": "inline@0"
362347
},
363348
{
364349
"command": "databricks.sync.stop",
365-
"when": "view == configurationView && viewItem =~ /^databricks.configuration.workspaceFsPath.(?!error|stopped|selected).*$/",
350+
"when": "view == configurationView && viewItem =~ /^databricks.configuration.sync.(?!error|stopped|selected).*$/",
366351
"group": "inline@0"
367352
},
368353
{

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

Lines changed: 1 addition & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Cluster, WorkspaceFsEntity, WorkspaceFsUtils} from "../sdk-extensions";
1+
import {Cluster} from "../sdk-extensions";
22
import {
33
Disposable,
44
FileSystemError,
@@ -16,7 +16,6 @@ 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 {RemoteUri, REPO_NAME_SUFFIX} from "../sync/SyncDestination";
2019
import {ConfigModel} from "./ConfigModel";
2120

2221
function formatQuickPickClusterSize(sizeInMB: number): string {
@@ -200,130 +199,6 @@ export class ConnectionCommands implements Disposable {
200199
};
201200
}
202201

203-
attachSyncDestinationCommand() {
204-
return async () => {
205-
const wsClient = this.connectionManager.workspaceClient;
206-
const me = this.connectionManager.databricksWorkspace?.userName;
207-
const rootDirPath =
208-
this.connectionManager.databricksWorkspace?.currentFsRoot;
209-
if (!wsClient || !me || !rootDirPath) {
210-
// TODO
211-
return;
212-
}
213-
214-
const rootDir = await WorkspaceFsEntity.fromPath(
215-
wsClient,
216-
rootDirPath.path
217-
);
218-
219-
type WorkspaceFsQuickPickItem = QuickPickItem & {
220-
path?: string;
221-
};
222-
const children: WorkspaceFsQuickPickItem[] = [
223-
{
224-
label: "Create New Sync Destination",
225-
alwaysShow: true,
226-
detail: workspaceConfigs.enableFilesInWorkspace
227-
? `Create a new folder under /Workspace/${me}/.ide as sync destination`
228-
: `Create a new Repo under /Repos/${me} as sync destination`,
229-
},
230-
{
231-
label: "Sync Destinations",
232-
kind: QuickPickItemKind.Separator,
233-
},
234-
];
235-
236-
const input = window.createQuickPick();
237-
input.busy = true;
238-
input.show();
239-
input.items = children;
240-
if (workspaceConfigs.enableFilesInWorkspace) {
241-
children.push(
242-
...((await rootDir?.children) ?? [])
243-
.filter((entity) =>
244-
WorkspaceFsUtils.isDirectory(entity)
245-
)
246-
.map((entity) => {
247-
return {
248-
label: entity.basename,
249-
detail: entity.path,
250-
path: entity.path,
251-
};
252-
})
253-
);
254-
} else {
255-
const repos = (await rootDir?.children) ?? [];
256-
257-
children.push(
258-
...repos
259-
.filter((entity) => {
260-
return entity.basename.endsWith(REPO_NAME_SUFFIX);
261-
})
262-
.map((entity) => {
263-
return {
264-
label: entity.basename.slice(
265-
0,
266-
-REPO_NAME_SUFFIX.length
267-
),
268-
detail: entity.path,
269-
path: entity.path,
270-
};
271-
})
272-
);
273-
}
274-
input.items = children;
275-
input.busy = false;
276-
277-
const disposables = [
278-
input,
279-
input.onDidAccept(async () => {
280-
const fn = async () => {
281-
const selection = input
282-
.selectedItems[0] as WorkspaceFsQuickPickItem;
283-
if (
284-
selection.label !== "Create New Sync Destination" &&
285-
selection.path
286-
) {
287-
this.connectionManager.attachSyncDestination(
288-
new RemoteUri(selection.path)
289-
);
290-
return;
291-
}
292-
const created =
293-
await this.wsfsCommands.createFolder(rootDir);
294-
if (created === undefined) {
295-
return;
296-
}
297-
this.connectionManager.attachSyncDestination(
298-
new RemoteUri(created.path)
299-
);
300-
};
301-
try {
302-
await fn();
303-
} catch (e: unknown) {
304-
if (e instanceof Error) {
305-
window.showErrorMessage(
306-
`Error while creating a new directory: ${e.message}`
307-
);
308-
}
309-
} finally {
310-
disposables.forEach((i) => i.dispose());
311-
}
312-
}),
313-
input.onDidHide(() => {
314-
disposables.forEach((i) => i.dispose());
315-
}),
316-
];
317-
};
318-
}
319-
320-
/**
321-
* Set workspace to undefined and remove workspace path from settings file.
322-
*/
323-
async detachWorkspaceCommand() {
324-
this.connectionManager.detachSyncDestination();
325-
}
326-
327202
async selectTarget() {
328203
const targets = await this.configModel.bundleConfigReaderWriter.targets;
329204
const currentTarget = this.configModel.target;

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,7 @@ export type DatabricksConfigSourceMap = {
1515
[key in keyof DatabricksConfig]: DatabricksConfigSource;
1616
};
1717

18-
export const OVERRIDEABLE_CONFIG_KEYS = [
19-
"clusterId",
20-
"authParams",
21-
"workspaceFsPath",
22-
] as const;
18+
export const OVERRIDEABLE_CONFIG_KEYS = ["clusterId", "authParams"] as const;
2319

2420
export type OverrideableConfig = Pick<
2521
DatabricksConfig,
@@ -55,6 +51,6 @@ export function isBundleConfigKey(key: any): key is keyof BundleConfig {
5551
}
5652

5753
export interface ConfigReaderWriter<T extends keyof DatabricksConfig> {
58-
read(key: T, target: string): Promise<DatabricksConfig[T] | undefined>;
54+
readAll(target: string): Promise<DatabricksConfig | undefined>;
5955
write(key: T, target: string, value?: DatabricksConfig[T]): Promise<void>;
6056
}

‎packages/databricks-vscode/src/configuration/ui/AuthTypeComponent.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import {BaseComponent} from "./BaseComponent";
44
import {ConfigurationTreeItem} from "./types";
55
import {ThemeIcon, ThemeColor} from "vscode";
66
const TREE_ICON_ID = "AUTH-TYPE";
7+
function getContextValue(key: string) {
8+
return `databricks.configuration.authType.${key}`;
9+
}
710

811
export class AuthTypeComponent extends BaseComponent {
912
constructor(
@@ -41,7 +44,7 @@ export class AuthTypeComponent extends BaseComponent {
4144
"account",
4245
new ThemeColor("notificationsErrorIcon.foreground")
4346
),
44-
contextValue: "databricks.configuration.authType.none",
47+
contextValue: getContextValue("none"),
4548
id: TREE_ICON_ID,
4649
command: {
4750
title: "Login to Databricks",
@@ -65,7 +68,7 @@ export class AuthTypeComponent extends BaseComponent {
6568
new ThemeColor("debugIcon.startForeground")
6669
),
6770
description: authProvider.describe(),
68-
contextValue: `databricks.configuration.authType.${authProvider.authType}`,
71+
contextValue: getContextValue(authProvider.authType),
6972
id: TREE_ICON_ID,
7073
source: config.source,
7174
},

‎packages/databricks-vscode/src/configuration/ui/ClusterComponent.ts‎

Lines changed: 23 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,17 @@ import {Cluster} from "../../sdk-extensions";
1212
import {onError} from "../../utils/onErrorDecorator";
1313

1414
const TREE_ICON_ID = "CLUSTER";
15+
function getContextValue(key: string) {
16+
return `databricks.configuration.cluster.${key}`;
17+
}
1518

1619
function getTreeItemsForClusterState(cluster: Cluster) {
1720
let icon, contextValue;
1821
switch (cluster.state) {
1922
case "RESIZING":
2023
case "RUNNING":
2124
icon = new ThemeIcon("debug-start");
22-
contextValue = "databricks.configuration.cluster.running";
25+
contextValue = getContextValue("running");
2326
break;
2427

2528
case "RESTARTING":
@@ -28,36 +31,36 @@ function getTreeItemsForClusterState(cluster: Cluster) {
2831
"sync~spin",
2932
new ThemeColor("debugIcon.startForeground")
3033
);
31-
contextValue = "databricks.configuration.cluster.pending";
34+
contextValue = getContextValue("pending");
3235
break;
3336

3437
case "TERMINATING":
3538
icon = new ThemeIcon(
3639
"sync~spin",
3740
new ThemeColor("notificationsErrorIcon.foreground")
3841
);
39-
contextValue = "databricks.configuration.cluster.terminating";
42+
contextValue = getContextValue("terminating");
4043
break;
4144

4245
case "TERMINATED":
4346
icon = new ThemeIcon(
4447
"stop-circle",
4548
new ThemeColor("notificationsErrorIcon.foreground")
4649
);
47-
contextValue = "databricks.configuration.cluster.terminated";
50+
contextValue = getContextValue("terminated");
4851
break;
4952

5053
case "ERROR":
5154
icon = new ThemeIcon(
5255
"testing-error-icon",
5356
new ThemeColor("notificationsErrorIcon.foreground")
5457
);
55-
contextValue = "databricks.configuration.cluster.error";
58+
contextValue = getContextValue("error");
5659
break;
5760

5861
default:
5962
icon = new ThemeIcon("question");
60-
contextValue = "databricks.configuration.cluster.unknown";
63+
contextValue = getContextValue("unknown");
6164
break;
6265
}
6366

@@ -86,13 +89,7 @@ export class ClusterComponent extends BaseComponent {
8689

8790
if (config?.config === undefined) {
8891
// Cluster is not set in bundle and override
89-
// We are not logged in
90-
if (this.connectionManager.state !== "CONNECTED") {
91-
return [];
92-
}
93-
94-
// Cluster is not set in bundle and override
95-
// We are logged in
92+
// We are logged in -> Select cluster prompt
9693
const label = "Select a cluster";
9794
return [
9895
{
@@ -101,7 +98,7 @@ export class ClusterComponent extends BaseComponent {
10198
highlights: [[0, label.length]],
10299
},
103100
collapsibleState: TreeItemCollapsibleState.Expanded,
104-
contextValue: "databricks.configuration.cluster.none",
101+
contextValue: getContextValue("none"),
105102
iconPath: new ThemeIcon(
106103
"server",
107104
new ThemeColor("notificationsErrorIcon.foreground")
@@ -111,29 +108,8 @@ export class ClusterComponent extends BaseComponent {
111108
];
112109
}
113110

114-
const {config: clusterId, source} = config;
115-
116111
// Cluster is set in bundle / override
117-
// We are not logged in
118-
if (this.connectionManager.state !== "CONNECTED") {
119-
return [
120-
{
121-
label: "Cluster",
122-
description: clusterId,
123-
collapsibleState: TreeItemCollapsibleState.Expanded,
124-
contextValue: "databricks.configuration.cluster.selected",
125-
iconPath: new ThemeIcon(
126-
"server",
127-
new ThemeColor("notificationsErrorIcon.foreground")
128-
),
129-
source: source,
130-
id: TREE_ICON_ID,
131-
},
132-
];
133-
}
134-
135-
// Cluster is set in bundle / override
136-
// We are logged in
112+
// We are logged in -> load cluster details
137113
const cluster = this.connectionManager.cluster;
138114
if (cluster === undefined) {
139115
// can never happen. Just need it for typescript type coercing.
@@ -148,7 +124,7 @@ export class ClusterComponent extends BaseComponent {
148124
collapsibleState: TreeItemCollapsibleState.Expanded,
149125
contextValue: contextValue,
150126
iconPath: icon,
151-
source: source,
127+
source: config.source,
152128
id: TREE_ICON_ID,
153129
},
154130
];
@@ -157,6 +133,14 @@ export class ClusterComponent extends BaseComponent {
157133
public async getChildren(
158134
parent?: TreeItem
159135
): Promise<ConfigurationTreeItem[]> {
136+
// Only show cluster details when we are connected and in dev mode. In other modes
137+
// there is no concept of a global interactive cluster
138+
if (
139+
this.connectionManager.state !== "CONNECTED" ||
140+
(await this.configModel.get("mode")) !== "dev"
141+
) {
142+
return [];
143+
}
160144
if (parent === undefined) {
161145
return this.getRoot();
162146
}
@@ -165,13 +149,10 @@ export class ClusterComponent extends BaseComponent {
165149
return [];
166150
}
167151

168-
if (
169-
this.connectionManager.state !== "CONNECTED" ||
170-
this.connectionManager.cluster === undefined
171-
) {
152+
// If there is no cluster, we don't have to show cluster details
153+
if (this.connectionManager.cluster === undefined) {
172154
return [];
173155
}
174-
175156
const cluster = this.connectionManager.cluster;
176157
const children: TreeItem[] = [
177158
{

0 commit comments

Comments
 (0)