Skip to content

Commit

Permalink
Fixes to initialisation UI (#1160)
Browse files Browse the repository at this point in the history
## 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? -->
  • Loading branch information
kartikgupta-db authored Apr 3, 2024
1 parent 2128013 commit 77267f4
Show file tree
Hide file tree
Showing 24 changed files with 286 additions and 159 deletions.
16 changes: 8 additions & 8 deletions packages/databricks-vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -313,22 +313,22 @@
"viewsWelcome": [
{
"view": "configurationView",
"contents": "Add Databricks configuration to the current project:\n[Initialize Project](command:databricks.bundle.startManualMigration)",
"when": "workspaceFolderCount > 0 && databricks.context.initialized && databricks.context.pendingManualMigration"
"contents": "There are multiple Databricks projects in the workspace:\n[Open existing Databricks Project](command:databricks.bundle.openSubProject)",
"when": "workspaceFolderCount > 0 && databricks.context.initialized && databricks.context.subProjectsAvailable"
},
{
"view": "configurationView",
"contents": "There are multiple Databricks projects in the workspace, please chose which one to open:\n[Open Existing Project](command:databricks.bundle.openSubProject)",
"when": "workspaceFolderCount > 0 && databricks.context.initialized && databricks.context.subProjectsAvailable"
"contents": "[Create a new Databricks Project](command:databricks.bundle.initNewProject)",
"when": "workspaceFolderCount > 0 && databricks.context.initialized"
},
{
"view": "configurationView",
"contents": "Create a new Databricks project?\n[Create a new Databricks Project](command:databricks.bundle.initNewProject)",
"when": "workspaceFolderCount > 0 && databricks.context.initialized && !databricks.context.pendingManualMigration"
"contents": "Migrate current workspace to a Databricks Project: \n[Migrate to Databricks Project](command:databricks.bundle.startManualMigration)",
"when": "workspaceFolderCount > 0 && databricks.context.initialized && databricks.context.pendingManualMigration"
},
{
"view": "configurationView",
"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).",
"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)",
"when": "databricks.context.initialized"
},
{
Expand All @@ -338,7 +338,7 @@
},
{
"view": "configurationView",
"contents": "The workspace is empty.\n[Initialize New Project](command:databricks.bundle.initNewProject)",
"contents": "The workspace is empty.\n[Create a new Databricks Project](command:databricks.bundle.initNewProject)",
"when": "workspaceFolderCount == 0"
}
],
Expand Down
21 changes: 11 additions & 10 deletions packages/databricks-vscode/src/bundle/BundleProjectManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,14 @@ export class BundleProjectManager {
const recordEvent = this.telemetry.start(
Events.EXTENSION_INITIALIZATION
);
await Promise.all([
// This method updates subProjectsAvailabe context.
// We have a configurationView that shows "openSubProjects" button if the context value is true.
this.detectSubProjects(),
// This method will try to automatically create bundle config if there's existing valid project.json config.
// In the case project.json doesn't exist or its auth doesn't work, it sets pendingManualMigration context
// to enable configurationView with the configureManualMigration button.
this.detectLegacyProjectConfig(),
]);
// This method updates subProjectsAvailabe context.
// We have a configurationView that shows "openSubProjects" button if the context value is true.
await this.detectSubProjects();
// This method will try to automatically create bundle config if there's existing valid project.json config.
// In the case project.json doesn't exist or its auth doesn't work, it sets pendingManualMigration context
// to enable configurationView with the configureManualMigration button.
await this.detectLegacyProjectConfig();

const type = this.legacyProjectConfig ? "legacy" : "unknown";
recordEvent({success: true, type});
}
Expand Down Expand Up @@ -156,7 +155,9 @@ export class BundleProjectManager {

private async detectLegacyProjectConfig() {
this.legacyProjectConfig = await this.loadLegacyProjectConfig();
if (!this.legacyProjectConfig) {
// If we have subprojects, we can't migrate automatically. We show the user option to
// manually migrate the project (create a new databricks.yml based on selected auth)
if (!this.legacyProjectConfig || (this.subProjects?.length ?? 0) > 0) {
this.customWhenContext.setPendingManualMigration(true);
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ export class BundlePreValidateModel extends BaseModelWithStateCache<BundlePreVal
});
}

public async setTarget(target: string | undefined) {
public setTarget(target: string | undefined) {
this.target = target;
await this.stateCache.refresh();
this.resetCache();
}

protected readStateFromTarget(
Expand Down Expand Up @@ -93,7 +93,6 @@ export class BundlePreValidateModel extends BaseModelWithStateCache<BundlePreVal
return targetObject;
}

@Mutex.synchronise("mutex")
protected async readState() {
if (this.target === undefined) {
return {};
Expand Down Expand Up @@ -139,6 +138,10 @@ export class BundlePreValidateModel extends BaseModelWithStateCache<BundlePreVal
return [...filesWithConfig, ...filesWithTarget][0];
}

public resetCache(): void {
this.stateCache.set({});
}

public dispose() {
this.disposables.forEach((d) => d.dispose());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,23 +97,20 @@ export class BundleRemoteStateModel extends BaseModelWithStateCache<BundleRemote
);
}

@Mutex.synchronise("mutex")
public async setTarget(target: string | undefined) {
public setTarget(target: string | undefined) {
if (this.target === target) {
return;
}
this.target = target;
this.resetCache();
this.authProvider = undefined;
await this.stateCache.refresh();
}

@Mutex.synchronise("mutex")
public async setAuthProvider(authProvider: AuthProvider | undefined) {
public setAuthProvider(authProvider: AuthProvider | undefined) {
if (
!lodash.isEqual(this.authProvider?.toJSON(), authProvider?.toJSON())
) {
this.authProvider = authProvider;
await this.stateCache.refresh();
}
}

Expand Down Expand Up @@ -146,6 +143,10 @@ export class BundleRemoteStateModel extends BaseModelWithStateCache<BundleRemote
}, resources);
}

public resetCache(): void {
this.stateCache.set({});
}

dispose() {
super.dispose();
if (this.refreshInterval !== undefined) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ export type BundleValidateState = {
} & BundleTarget;

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

Expand All @@ -40,23 +40,20 @@ export class BundleValidateModel extends BaseModelWithStateCache<BundleValidateS
);
}

@Mutex.synchronise("mutex")
public async setTarget(target: string | undefined) {
public setTarget(target: string | undefined) {
if (this.target === target) {
return;
}
this.target = target;
this.resetCache();
this.authProvider = undefined;
await this.stateCache.refresh();
}

@Mutex.synchronise("mutex")
public async setAuthProvider(authProvider: AuthProvider | undefined) {
public setAuthProvider(authProvider: AuthProvider | undefined) {
if (
!lodash.isEqual(this.authProvider?.toJSON(), authProvider?.toJSON())
) {
this.authProvider = authProvider;
await this.stateCache.refresh();
}
}

Expand All @@ -82,6 +79,10 @@ export class BundleValidateModel extends BaseModelWithStateCache<BundleValidateS
};
}

public resetCache(): void {
this.stateCache.set({});
}

dispose() {
this.disposables.forEach((i) => i.dispose());
}
Expand Down
14 changes: 14 additions & 0 deletions packages/databricks-vscode/src/cli/CliWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ export class ProcessError extends Error {
) {
super(message);
}

showErrorMessage(prefix?: string) {
window
.showErrorMessage(
(prefix?.trimEnd().concat(" ") ?? "") +
`Error executing Databricks CLI command.`,
"Show Logs"
)
.then((choice) => {
if (choice === "Show Logs") {
commands.executeCommand("databricks.bundle.showLogs");
}
});
}
}

async function waitForProcess(
Expand Down
13 changes: 11 additions & 2 deletions packages/databricks-vscode/src/configuration/ConnectionCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import {ConfigModel} from "./models/ConfigModel";
import {saveNewProfile} from "./LoginWizard";
import {PersonalAccessTokenAuthProvider} from "./auth/AuthProvider";
import {normalizeHost} from "../utils/urlUtils";
import {CliWrapper} from "../cli/CliWrapper";
import {CliWrapper, ProcessError} from "../cli/CliWrapper";
import {AUTH_TYPE_SWITCH_ID, AUTH_TYPE_LOGIN_ID} from "./ui/AuthTypeComponent";
import {ManualLoginSource} from "../telemetry/constants";
import {onError} from "../utils/onErrorDecorator";

function formatQuickPickClusterSize(sizeInMB: number): string {
if (sizeInMB > 1024) {
Expand Down Expand Up @@ -204,6 +205,7 @@ export class ConnectionCommands implements Disposable {
};
}

@onError({popup: {prefix: "Error selecting target."}})
async selectTarget() {
const targets = await this.configModel.targets;
const currentTarget = this.configModel.target;
Expand All @@ -226,7 +228,14 @@ export class ConnectionCommands implements Disposable {
if (selectedTarget === undefined) {
return;
}
this.configModel.setTarget(selectedTarget.label);
try {
await this.configModel.setTarget(selectedTarget.label);
} catch (e) {
if (e instanceof ProcessError) {
e.showErrorMessage("Error selecting target");
}
throw e;
}
}

dispose() {
Expand Down
35 changes: 13 additions & 22 deletions packages/databricks-vscode/src/configuration/ConnectionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import {
AuthType as SdkAuthType,
} from "@databricks/databricks-sdk";
import {Cluster} from "../sdk-extensions";
import {EventEmitter, Uri, window, Disposable, commands} from "vscode";
import {EventEmitter, Uri, window, Disposable} from "vscode";
import {CliWrapper, ProcessError} from "../cli/CliWrapper";
import {
SyncDestinationMapper,
RemoteUri,
LocalUri,
} from "../sync/SyncDestination";
import {LoginWizard, listProfiles} from "./LoginWizard";
import {LoginWizard, getProfilesForHost} from "./LoginWizard";
import {ClusterManager} from "../cluster/ClusterManager";
import {DatabricksWorkspace} from "./DatabricksWorkspace";
import {CustomWhenContext} from "../vscode-objs/CustomWhenContext";
Expand Down Expand Up @@ -207,6 +207,7 @@ export class ConnectionManager implements Disposable {
}
recordEvent({success: this.state === "CONNECTED", source});
} catch (e) {
await this.disconnect();
recordEvent({success: false, source});
throw e;
}
Expand Down Expand Up @@ -246,9 +247,7 @@ export class ConnectionManager implements Disposable {
}

// Try to load a unique profile that matches the host
const profiles = (await listProfiles(this.cli)).filter(
(p) => p.host?.toString() === host.toString()
);
const profiles = await getProfilesForHost(host, this.cli);
if (profiles.length !== 1) {
return;
}
Expand All @@ -272,28 +271,16 @@ export class ConnectionManager implements Disposable {
);
} catch (e) {
NamedLogger.getOrCreate("Extension").error(
`Can't connect to the workspace`,
`Error connecting to the workspace`,
e
);
if (e instanceof ProcessError) {
window
.showErrorMessage(
`Can't connect to the workspace. Error executing Databricks CLI command.`,
"Show Logs"
)
.then((choice) => {
if (choice === "Show Logs") {
commands.executeCommand(
"databricks.bundle.showLogs"
);
}
});
e.showErrorMessage("Error connecting to the workspace.");
} else if (e instanceof Error) {
window.showErrorMessage(
`Can't connect to the workspace: "${e.message}"."`
`Error connecting to the workspace: "${e.message}"."`
);
}
await this.logout();
}
}

Expand All @@ -309,11 +296,15 @@ export class ConnectionManager implements Disposable {
"authProfile",
authProvider.toJSON().profile as string | undefined
);
await this.configModel.setAuthProvider(authProvider);

await this.updateSyncDestinationMapper();
await this.updateClusterManager();
await this._metadataService.setApiClient(this.apiClient);
this.updateState("CONNECTED");
try {
await this.configModel.setAuthProvider(authProvider);
} finally {
this.updateState("CONNECTED");
}
}

@Mutex.synchronise("loginLogoutMutex")
Expand Down
8 changes: 7 additions & 1 deletion packages/databricks-vscode/src/configuration/LoginWizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ export async function listProfiles(cliWrapper: CliWrapper) {
async () => {
const profiles = (
await cliWrapper.listProfiles(
workspaceConfigs.databrickscfgLocation
FileUtils.getDatabricksConfigFilePath().fsPath
)
).filter((profile) => {
try {
Expand All @@ -456,6 +456,12 @@ export async function listProfiles(cliWrapper: CliWrapper) {
);
}

export async function getProfilesForHost(host: URL, cliWrapper: CliWrapper) {
return (await listProfiles(cliWrapper)).filter(
(profile) => profile.host?.toString() === host.toString()
);
}

async function validateDatabricksHost(
host: string
): Promise<string | undefined | ValidationMessageType> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ export class ProfileAuthProvider extends AuthProvider {

constructor(
host: URL,
private readonly profile: string,
readonly profile: string,
private readonly cli: CliWrapper,
checked = false
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ export abstract class BaseModelWithStateCache<StateType extends object>
}>;
}

public refresh() {
this.stateCache.refresh();
}

public abstract resetCache(): void;
dispose() {
this.disposables.forEach((i) => i.dispose());
}
Expand Down
Loading

0 comments on commit 77267f4

Please sign in to comment.