Skip to content

Commit 77267f4

Browse files
kartikgupta-dbkartikgupta-db
authored
Fixes to initialisation UI (#1160)
## Changes ### Welcome view changes * Always show button to create a databricks project. * Don't automatically migrate from old project.json if there are subprojects present. * Reword welcome view buttons and text <img width="528" alt="image" src="https://github.com/databricks/databricks-vscode/assets/88345179/f8624887-6cdc-4ad7-97eb-c46ad9b8b42e"> ### Login Changes * Don't logout if refreshing one of the models fails. This means users can see the error, fix it and have their fixes automatically be picked up and tried out. * Show "Multiple login profiles available. Click to select a profile." when multiple profiles found in .databrickscfg. ## Tests <!-- How is this tested? -->
1 parent 2128013 commit 77267f4

24 files changed

+286
-159
lines changed

‎packages/databricks-vscode/package.json‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -313,22 +313,22 @@
313313
"viewsWelcome": [
314314
{
315315
"view": "configurationView",
316-
"contents": "Add Databricks configuration to the current project:\n[Initialize Project](command:databricks.bundle.startManualMigration)",
317-
"when": "workspaceFolderCount > 0 && databricks.context.initialized && databricks.context.pendingManualMigration"
316+
"contents": "There are multiple Databricks projects in the workspace:\n[Open existing Databricks Project](command:databricks.bundle.openSubProject)",
317+
"when": "workspaceFolderCount > 0 && databricks.context.initialized && databricks.context.subProjectsAvailable"
318318
},
319319
{
320320
"view": "configurationView",
321-
"contents": "There are multiple Databricks projects in the workspace, please chose which one to open:\n[Open Existing Project](command:databricks.bundle.openSubProject)",
322-
"when": "workspaceFolderCount > 0 && databricks.context.initialized && databricks.context.subProjectsAvailable"
321+
"contents": "[Create a new Databricks Project](command:databricks.bundle.initNewProject)",
322+
"when": "workspaceFolderCount > 0 && databricks.context.initialized"
323323
},
324324
{
325325
"view": "configurationView",
326-
"contents": "Create a new Databricks project?\n[Create a new Databricks Project](command:databricks.bundle.initNewProject)",
327-
"when": "workspaceFolderCount > 0 && databricks.context.initialized && !databricks.context.pendingManualMigration"
326+
"contents": "Migrate current workspace to a Databricks Project: \n[Migrate to Databricks Project](command:databricks.bundle.startManualMigration)",
327+
"when": "workspaceFolderCount > 0 && databricks.context.initialized && databricks.context.pendingManualMigration"
328328
},
329329
{
330330
"view": "configurationView",
331-
"contents": "[Show Quickstart](command:databricks.quickstart.open)\nTo learn more about how to use Databricks with VS Code [read our docs](https://docs.databricks.com/dev-tools/vscode-ext.html).",
331+
"contents": "To learn more about how to use Databricks with VS Code [read our docs](https://docs.databricks.com/dev-tools/vscode-ext.html) or [Quickstart guide](command:databricks.quickstart.open)",
332332
"when": "databricks.context.initialized"
333333
},
334334
{
@@ -338,7 +338,7 @@
338338
},
339339
{
340340
"view": "configurationView",
341-
"contents": "The workspace is empty.\n[Initialize New Project](command:databricks.bundle.initNewProject)",
341+
"contents": "The workspace is empty.\n[Create a new Databricks Project](command:databricks.bundle.initNewProject)",
342342
"when": "workspaceFolderCount == 0"
343343
}
344344
],

‎packages/databricks-vscode/src/bundle/BundleProjectManager.ts‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,14 @@ export class BundleProjectManager {
8787
const recordEvent = this.telemetry.start(
8888
Events.EXTENSION_INITIALIZATION
8989
);
90-
await Promise.all([
91-
// This method updates subProjectsAvailabe context.
92-
// We have a configurationView that shows "openSubProjects" button if the context value is true.
93-
this.detectSubProjects(),
94-
// This method will try to automatically create bundle config if there's existing valid project.json config.
95-
// In the case project.json doesn't exist or its auth doesn't work, it sets pendingManualMigration context
96-
// to enable configurationView with the configureManualMigration button.
97-
this.detectLegacyProjectConfig(),
98-
]);
90+
// This method updates subProjectsAvailabe context.
91+
// We have a configurationView that shows "openSubProjects" button if the context value is true.
92+
await this.detectSubProjects();
93+
// This method will try to automatically create bundle config if there's existing valid project.json config.
94+
// In the case project.json doesn't exist or its auth doesn't work, it sets pendingManualMigration context
95+
// to enable configurationView with the configureManualMigration button.
96+
await this.detectLegacyProjectConfig();
97+
9998
const type = this.legacyProjectConfig ? "legacy" : "unknown";
10099
recordEvent({success: true, type});
101100
}
@@ -156,7 +155,9 @@ export class BundleProjectManager {
156155

157156
private async detectLegacyProjectConfig() {
158157
this.legacyProjectConfig = await this.loadLegacyProjectConfig();
159-
if (!this.legacyProjectConfig) {
158+
// If we have subprojects, we can't migrate automatically. We show the user option to
159+
// manually migrate the project (create a new databricks.yml based on selected auth)
160+
if (!this.legacyProjectConfig || (this.subProjects?.length ?? 0) > 0) {
160161
this.customWhenContext.setPendingManualMigration(true);
161162
return;
162163
}

‎packages/databricks-vscode/src/bundle/models/BundlePreValidateModel.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ export class BundlePreValidateModel extends BaseModelWithStateCache<BundlePreVal
6363
});
6464
}
6565

66-
public async setTarget(target: string | undefined) {
66+
public setTarget(target: string | undefined) {
6767
this.target = target;
68-
await this.stateCache.refresh();
68+
this.resetCache();
6969
}
7070

7171
protected readStateFromTarget(
@@ -93,7 +93,6 @@ export class BundlePreValidateModel extends BaseModelWithStateCache<BundlePreVal
9393
return targetObject;
9494
}
9595

96-
@Mutex.synchronise("mutex")
9796
protected async readState() {
9897
if (this.target === undefined) {
9998
return {};
@@ -139,6 +138,10 @@ export class BundlePreValidateModel extends BaseModelWithStateCache<BundlePreVal
139138
return [...filesWithConfig, ...filesWithTarget][0];
140139
}
141140

141+
public resetCache(): void {
142+
this.stateCache.set({});
143+
}
144+
142145
public dispose() {
143146
this.disposables.forEach((d) => d.dispose());
144147
}

‎packages/databricks-vscode/src/bundle/models/BundleRemoteStateModel.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,23 +97,20 @@ export class BundleRemoteStateModel extends BaseModelWithStateCache<BundleRemote
9797
);
9898
}
9999

100-
@Mutex.synchronise("mutex")
101-
public async setTarget(target: string | undefined) {
100+
public setTarget(target: string | undefined) {
102101
if (this.target === target) {
103102
return;
104103
}
105104
this.target = target;
105+
this.resetCache();
106106
this.authProvider = undefined;
107-
await this.stateCache.refresh();
108107
}
109108

110-
@Mutex.synchronise("mutex")
111-
public async setAuthProvider(authProvider: AuthProvider | undefined) {
109+
public setAuthProvider(authProvider: AuthProvider | undefined) {
112110
if (
113111
!lodash.isEqual(this.authProvider?.toJSON(), authProvider?.toJSON())
114112
) {
115113
this.authProvider = authProvider;
116-
await this.stateCache.refresh();
117114
}
118115
}
119116

@@ -146,6 +143,10 @@ export class BundleRemoteStateModel extends BaseModelWithStateCache<BundleRemote
146143
}, resources);
147144
}
148145

146+
public resetCache(): void {
147+
this.stateCache.set({});
148+
}
149+
149150
dispose() {
150151
super.dispose();
151152
if (this.refreshInterval !== undefined) {

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ export type BundleValidateState = {
1717
} & BundleTarget;
1818

1919
export class BundleValidateModel extends BaseModelWithStateCache<BundleValidateState> {
20-
private target: string | undefined;
21-
private authProvider: AuthProvider | undefined;
20+
public target: string | undefined;
21+
public authProvider: AuthProvider | undefined;
2222
protected mutex = new Mutex();
2323
protected logger = logging.NamedLogger.getOrCreate(Loggers.Bundle);
2424

@@ -40,23 +40,20 @@ export class BundleValidateModel extends BaseModelWithStateCache<BundleValidateS
4040
);
4141
}
4242

43-
@Mutex.synchronise("mutex")
44-
public async setTarget(target: string | undefined) {
43+
public setTarget(target: string | undefined) {
4544
if (this.target === target) {
4645
return;
4746
}
4847
this.target = target;
48+
this.resetCache();
4949
this.authProvider = undefined;
50-
await this.stateCache.refresh();
5150
}
5251

53-
@Mutex.synchronise("mutex")
54-
public async setAuthProvider(authProvider: AuthProvider | undefined) {
52+
public setAuthProvider(authProvider: AuthProvider | undefined) {
5553
if (
5654
!lodash.isEqual(this.authProvider?.toJSON(), authProvider?.toJSON())
5755
) {
5856
this.authProvider = authProvider;
59-
await this.stateCache.refresh();
6057
}
6158
}
6259

@@ -82,6 +79,10 @@ export class BundleValidateModel extends BaseModelWithStateCache<BundleValidateS
8279
};
8380
}
8481

82+
public resetCache(): void {
83+
this.stateCache.set({});
84+
}
85+
8586
dispose() {
8687
this.disposables.forEach((i) => i.dispose());
8788
}

‎packages/databricks-vscode/src/cli/CliWrapper.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,20 @@ export class ProcessError extends Error {
4444
) {
4545
super(message);
4646
}
47+
48+
showErrorMessage(prefix?: string) {
49+
window
50+
.showErrorMessage(
51+
(prefix?.trimEnd().concat(" ") ?? "") +
52+
`Error executing Databricks CLI command.`,
53+
"Show Logs"
54+
)
55+
.then((choice) => {
56+
if (choice === "Show Logs") {
57+
commands.executeCommand("databricks.bundle.showLogs");
58+
}
59+
});
60+
}
4761
}
4862

4963
async function waitForProcess(

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@ import {ConfigModel} from "./models/ConfigModel";
1515
import {saveNewProfile} from "./LoginWizard";
1616
import {PersonalAccessTokenAuthProvider} from "./auth/AuthProvider";
1717
import {normalizeHost} from "../utils/urlUtils";
18-
import {CliWrapper} from "../cli/CliWrapper";
18+
import {CliWrapper, ProcessError} from "../cli/CliWrapper";
1919
import {AUTH_TYPE_SWITCH_ID, AUTH_TYPE_LOGIN_ID} from "./ui/AuthTypeComponent";
2020
import {ManualLoginSource} from "../telemetry/constants";
21+
import {onError} from "../utils/onErrorDecorator";
2122

2223
function formatQuickPickClusterSize(sizeInMB: number): string {
2324
if (sizeInMB > 1024) {
@@ -204,6 +205,7 @@ export class ConnectionCommands implements Disposable {
204205
};
205206
}
206207

208+
@onError({popup: {prefix: "Error selecting target."}})
207209
async selectTarget() {
208210
const targets = await this.configModel.targets;
209211
const currentTarget = this.configModel.target;
@@ -226,7 +228,14 @@ export class ConnectionCommands implements Disposable {
226228
if (selectedTarget === undefined) {
227229
return;
228230
}
229-
this.configModel.setTarget(selectedTarget.label);
231+
try {
232+
await this.configModel.setTarget(selectedTarget.label);
233+
} catch (e) {
234+
if (e instanceof ProcessError) {
235+
e.showErrorMessage("Error selecting target");
236+
}
237+
throw e;
238+
}
230239
}
231240

232241
dispose() {

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

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ import {
55
AuthType as SdkAuthType,
66
} from "@databricks/databricks-sdk";
77
import {Cluster} from "../sdk-extensions";
8-
import {EventEmitter, Uri, window, Disposable, commands} from "vscode";
8+
import {EventEmitter, Uri, window, Disposable} from "vscode";
99
import {CliWrapper, ProcessError} from "../cli/CliWrapper";
1010
import {
1111
SyncDestinationMapper,
1212
RemoteUri,
1313
LocalUri,
1414
} from "../sync/SyncDestination";
15-
import {LoginWizard, listProfiles} from "./LoginWizard";
15+
import {LoginWizard, getProfilesForHost} from "./LoginWizard";
1616
import {ClusterManager} from "../cluster/ClusterManager";
1717
import {DatabricksWorkspace} from "./DatabricksWorkspace";
1818
import {CustomWhenContext} from "../vscode-objs/CustomWhenContext";
@@ -207,6 +207,7 @@ export class ConnectionManager implements Disposable {
207207
}
208208
recordEvent({success: this.state === "CONNECTED", source});
209209
} catch (e) {
210+
await this.disconnect();
210211
recordEvent({success: false, source});
211212
throw e;
212213
}
@@ -246,9 +247,7 @@ export class ConnectionManager implements Disposable {
246247
}
247248

248249
// Try to load a unique profile that matches the host
249-
const profiles = (await listProfiles(this.cli)).filter(
250-
(p) => p.host?.toString() === host.toString()
251-
);
250+
const profiles = await getProfilesForHost(host, this.cli);
252251
if (profiles.length !== 1) {
253252
return;
254253
}
@@ -272,28 +271,16 @@ export class ConnectionManager implements Disposable {
272271
);
273272
} catch (e) {
274273
NamedLogger.getOrCreate("Extension").error(
275-
`Can't connect to the workspace`,
274+
`Error connecting to the workspace`,
276275
e
277276
);
278277
if (e instanceof ProcessError) {
279-
window
280-
.showErrorMessage(
281-
`Can't connect to the workspace. Error executing Databricks CLI command.`,
282-
"Show Logs"
283-
)
284-
.then((choice) => {
285-
if (choice === "Show Logs") {
286-
commands.executeCommand(
287-
"databricks.bundle.showLogs"
288-
);
289-
}
290-
});
278+
e.showErrorMessage("Error connecting to the workspace.");
291279
} else if (e instanceof Error) {
292280
window.showErrorMessage(
293-
`Can't connect to the workspace: "${e.message}"."`
281+
`Error connecting to the workspace: "${e.message}"."`
294282
);
295283
}
296-
await this.logout();
297284
}
298285
}
299286

@@ -309,11 +296,15 @@ export class ConnectionManager implements Disposable {
309296
"authProfile",
310297
authProvider.toJSON().profile as string | undefined
311298
);
312-
await this.configModel.setAuthProvider(authProvider);
299+
313300
await this.updateSyncDestinationMapper();
314301
await this.updateClusterManager();
315302
await this._metadataService.setApiClient(this.apiClient);
316-
this.updateState("CONNECTED");
303+
try {
304+
await this.configModel.setAuthProvider(authProvider);
305+
} finally {
306+
this.updateState("CONNECTED");
307+
}
317308
}
318309

319310
@Mutex.synchronise("loginLogoutMutex")

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ export async function listProfiles(cliWrapper: CliWrapper) {
440440
async () => {
441441
const profiles = (
442442
await cliWrapper.listProfiles(
443-
workspaceConfigs.databrickscfgLocation
443+
FileUtils.getDatabricksConfigFilePath().fsPath
444444
)
445445
).filter((profile) => {
446446
try {
@@ -456,6 +456,12 @@ export async function listProfiles(cliWrapper: CliWrapper) {
456456
);
457457
}
458458

459+
export async function getProfilesForHost(host: URL, cliWrapper: CliWrapper) {
460+
return (await listProfiles(cliWrapper)).filter(
461+
(profile) => profile.host?.toString() === host.toString()
462+
);
463+
}
464+
459465
async function validateDatabricksHost(
460466
host: string
461467
): Promise<string | undefined | ValidationMessageType> {

‎packages/databricks-vscode/src/configuration/auth/AuthProvider.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ export class ProfileAuthProvider extends AuthProvider {
188188

189189
constructor(
190190
host: URL,
191-
private readonly profile: string,
191+
readonly profile: string,
192192
private readonly cli: CliWrapper,
193193
checked = false
194194
) {

0 commit comments

Comments
 (0)