From e2fb4a3a164fbc800696793a23d38f7737be9999 Mon Sep 17 00:00:00 2001 From: Ilia Babanov Date: Thu, 10 Oct 2024 10:46:37 +0200 Subject: [PATCH] Try to get saved profile from the legacy config (#1387) ## Changes V2 extension currently requires manual action from users if we detect multiple profiles that match the selected target. But in the V1 users might have already disambiguated the profiles, so here we use this inromation to get the saved profile. ## Tests Manually --- .../src/bundle/BundleFileSet.test.ts | 4 ++-- .../src/bundle/BundleProjectManager.ts | 16 +++++++++++++--- .../src/cli/CliWrapper.test.ts | 4 +++- .../src/configuration/ConnectionManager.ts | 18 +++++++++++++++++- .../src/file-managers/ProjectConfigFile.ts | 19 +++++++++++++------ .../src/telemetry/constants.ts | 9 +++++++++ 6 files changed, 57 insertions(+), 13 deletions(-) diff --git a/packages/databricks-vscode/src/bundle/BundleFileSet.test.ts b/packages/databricks-vscode/src/bundle/BundleFileSet.test.ts index 3772b04e2..b1a99a621 100644 --- a/packages/databricks-vscode/src/bundle/BundleFileSet.test.ts +++ b/packages/databricks-vscode/src/bundle/BundleFileSet.test.ts @@ -113,8 +113,8 @@ describe(__filename, async function () { path.join(tmpdirUri.fsPath, "includes", "included.yaml") ), ].map((v) => v.fsPath); - expect(Array.from(new Set(actual).values())).to.deep.equal( - Array.from(new Set(expected).values()) + expect(Array.from(new Set(actual).values()).sort()).to.deep.equal( + Array.from(new Set(expected).values()).sort() ); }); diff --git a/packages/databricks-vscode/src/bundle/BundleProjectManager.ts b/packages/databricks-vscode/src/bundle/BundleProjectManager.ts index bda7d8e9a..daba1b77c 100644 --- a/packages/databricks-vscode/src/bundle/BundleProjectManager.ts +++ b/packages/databricks-vscode/src/bundle/BundleProjectManager.ts @@ -155,6 +155,9 @@ export class BundleProjectManager { this.customWhenContext.setSubProjectsAvailable( this.subProjects?.length > 0 ); + this.telemetry.recordEvent(Events.BUNDLE_SUB_PROJECTS, { + count: this.subProjects?.length ?? 0, + }); } public async openSubProjects() { @@ -163,12 +166,19 @@ export class BundleProjectManager { } } + private setPendingManualMigration() { + this.customWhenContext.setPendingManualMigration(true); + this.telemetry.recordEvent(Events.CONNECTION_STATE_CHANGED, { + newState: "PENDING_MANUAL_MIGRATION", + }); + } + private async detectLegacyProjectConfig() { this.legacyProjectConfig = await this.loadLegacyProjectConfig(); // 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); + this.setPendingManualMigration(); return; } this.logger.debug( @@ -182,7 +192,7 @@ export class BundleProjectManager { ); } catch (error) { recordEvent({success: false}); - this.customWhenContext.setPendingManualMigration(true); + this.setPendingManualMigration(); const message = "Failed to perform automatic migration to Databricks Asset Bundles."; this.logger.error(message, error); @@ -214,7 +224,7 @@ export class BundleProjectManager { this.logger.debug( "Legacy project auth was not successful, showing 'configure' welcome screen" ); - this.customWhenContext.setPendingManualMigration(true); + this.setPendingManualMigration(); recordEvent({success: false}); return; } diff --git a/packages/databricks-vscode/src/cli/CliWrapper.test.ts b/packages/databricks-vscode/src/cli/CliWrapper.test.ts index 8f1f5f822..913e55078 100644 --- a/packages/databricks-vscode/src/cli/CliWrapper.test.ts +++ b/packages/databricks-vscode/src/cli/CliWrapper.test.ts @@ -50,7 +50,9 @@ function createCliWrapper(logFilePath?: string) { ); } -describe(__filename, () => { +describe(__filename, function () { + this.timeout("10s"); + it("should embed a working databricks CLI", async () => { const result = await execFile(cliPath, ["--help"]); assert.ok(result.stdout.indexOf("databricks") > 0); diff --git a/packages/databricks-vscode/src/configuration/ConnectionManager.ts b/packages/databricks-vscode/src/configuration/ConnectionManager.ts index 123300662..6caae9104 100644 --- a/packages/databricks-vscode/src/configuration/ConnectionManager.ts +++ b/packages/databricks-vscode/src/configuration/ConnectionManager.ts @@ -25,6 +25,7 @@ import {Events, Telemetry} from "../telemetry"; import {AutoLoginSource, ManualLoginSource} from "../telemetry/constants"; import {Barrier} from "../locking/Barrier"; import {WorkspaceFolderManager} from "../vscode-objs/WorkspaceFolderManager"; +import {ProjectConfigFile} from "../file-managers/ProjectConfigFile"; // eslint-disable-next-line @typescript-eslint/naming-convention const {NamedLogger} = logging; @@ -256,6 +257,16 @@ export class ConnectionManager implements Disposable { }); } + private async loadLegacyProjectConfig() { + try { + return await ProjectConfigFile.loadConfig(this.workspaceUri.fsPath); + } catch (error) { + const logger = NamedLogger.getOrCreate("Extension"); + logger.error(`Error loading legacy config`, error); + return undefined; + } + } + @onError({popup: {prefix: "Failed to login."}}) @Mutex.synchronise("loginLogoutMutex") private async resolveAuth() { @@ -267,8 +278,13 @@ export class ConnectionManager implements Disposable { } // Try to load a profile user had previously selected for this target - const savedProfile = (await this.configModel.get("overrides")) + let savedProfile = (await this.configModel.get("overrides")) ?.authProfile; + // Check if the profile is saved in the legacy project.json file + if (!savedProfile) { + const legacyConfig = await this.loadLegacyProjectConfig(); + savedProfile = legacyConfig?.profile; + } if (savedProfile !== undefined) { const authProvider = await ProfileAuthProvider.from( savedProfile, diff --git a/packages/databricks-vscode/src/file-managers/ProjectConfigFile.ts b/packages/databricks-vscode/src/file-managers/ProjectConfigFile.ts index b8822d854..234685d55 100644 --- a/packages/databricks-vscode/src/file-managers/ProjectConfigFile.ts +++ b/packages/databricks-vscode/src/file-managers/ProjectConfigFile.ts @@ -53,16 +53,14 @@ export class ProjectConfigFile { return await ProfileAuthProvider.from(config.profile, cli); } - static async load( - rootPath: string, - cli: CliWrapper - ): Promise { + static async loadConfig( + rootPath: string + ): Promise | undefined> { const projectConfigFilePath = path.join( path.normalize(rootPath), ".databricks", "project.json" ); - let rawConfig; try { rawConfig = await fs.readFile(projectConfigFilePath, { @@ -75,9 +73,18 @@ export class ProjectConfigFile { throw error; } } + return JSON.parse(rawConfig); + } + static async load( + rootPath: string, + cli: CliWrapper + ): Promise { + const config = await ProjectConfigFile.loadConfig(rootPath); + if (!config) { + return undefined; + } let authProvider: AuthProvider; - const config = JSON.parse(rawConfig); if (!config.authType && config.profile) { authProvider = await this.importOldConfig(config, cli); } else { diff --git a/packages/databricks-vscode/src/telemetry/constants.ts b/packages/databricks-vscode/src/telemetry/constants.ts index f786fc0ed..3a44b450c 100644 --- a/packages/databricks-vscode/src/telemetry/constants.ts +++ b/packages/databricks-vscode/src/telemetry/constants.ts @@ -18,6 +18,7 @@ export enum Events { MANUAL_MIGRATION = "manualMigration", BUNDLE_RUN = "bundleRun", BUNDLE_INIT = "bundleInit", + BUNDLE_SUB_PROJECTS = "bundleSubProjects", CONNECTION_STATE_CHANGED = "connectionStateChanged", } /* eslint-enable @typescript-eslint/naming-convention */ @@ -139,6 +140,14 @@ export class EventTypes { > = { comment: "Initialize a new bundle project", }; + [Events.BUNDLE_SUB_PROJECTS]: EventType<{ + count: number; + }> = { + comment: "Sub-projects in the active workspace folder", + count: { + comment: "Amount of sub-projects in the active workspace folder", + }, + }; [Events.CONNECTION_STATE_CHANGED]: EventType<{ newState: string; }> = {