All AuthProviders must implement a non silent check. (#939)

## Changes
* Moving forward, we want to avoid having silent checks to make
reasoning about auth easier. This PR makes it so that all AuthProviders
must implement the `check` method.
* Each check method must handle it's own Retry Loop, error handling and
display. Since now checks are always interactive, we do not want calling
code to manage the interactions.

## Tests
* manual

Author: kartikgupta-db

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

@@ -142,7 +142,7 @@ export class ConnectionManager {
                 }
             }
 
-            if (!(await projectConfigFile.authProvider.check(true))) {
+            if (!(await projectConfigFile.authProvider.check())) {
                 throw new Error(
                     `Can't login with ${projectConfigFile.authProvider.describe()}.`
                 );
@@ -277,7 +277,7 @@ export class ConnectionManager {
                 return;
             }
 
-            if (!(await config.authProvider.check(false))) {
+            if (!(await config.authProvider.check())) {
                 return;
             }
 

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

@@ -3,7 +3,9 @@ import {
     Config,
     ProductVersion,
     WorkspaceClient,
+    logging,
 } from "@databricks/databricks-sdk";
+import {window} from "vscode";
 import {normalizeHost} from "../../utils/urlUtils";
 import {workspaceConfigs} from "../../vscode-objs/WorkspaceConfigs";
 
@@ -13,6 +15,7 @@ const extensionVersion = require("../../../package.json")
 
 import {AzureCliCheck} from "./AzureCliCheck";
 import {DatabricksCliCheck} from "./DatabricksCliCheck";
+import {Loggers} from "../../logger";
 
 // TODO: Resolve this with SDK's AuthType.
 export type AuthType = "azure-cli" | "google-id" | "databricks-cli" | "profile";
@@ -47,10 +50,12 @@ export abstract class AuthProvider {
         });
     }
 
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    async check(silent: boolean): Promise<boolean> {
-        return true;
-    }
+    /**
+     * Check if the currently selected auth method can be used to login to Databricks.
+     * This function should not throw an error and each implementing class must
+     * handle it's own error messages and retry loops.
+     */
+    abstract check(): Promise<boolean>;
 
     protected abstract getSdkConfig(): Config;
 
@@ -129,6 +134,25 @@ export class ProfileAuthProvider extends AuthProvider {
             env: {},
         });
     }
+
+    async check() {
+        try {
+            const workspaceClient = this.getWorkspaceClient();
+            await workspaceClient.currentUser.me();
+            return true;
+        } catch (e) {
+            let message: string = `Can't login with config profile ${this.profile}`;
+            if (e instanceof Error) {
+                message = `Can't login with config profile ${this.profile}: ${e.message}`;
+            }
+            logging.NamedLogger.getOrCreate(Loggers.Extension).error(
+                message,
+                e
+            );
+            window.showErrorMessage(message);
+            return false;
+        }
+    }
 }
 
 export class DatabricksCliAuthProvider extends AuthProvider {
@@ -167,9 +191,9 @@ export class DatabricksCliAuthProvider extends AuthProvider {
         };
     }
 
-    async check(silent: boolean): Promise<boolean> {
+    async check(): Promise<boolean> {
         const databricksCliCheck = new DatabricksCliCheck(this);
-        return databricksCliCheck.check(silent);
+        return databricksCliCheck.check();
     }
 }
 
@@ -225,9 +249,9 @@ export class AzureCliAuthProvider extends AuthProvider {
         return envVars;
     }
 
-    async check(silent: boolean): Promise<boolean> {
+    async check(): Promise<boolean> {
         const cliCheck = new AzureCliCheck(this);
-        const result = await cliCheck.check(silent);
+        const result = await cliCheck.check();
         this._tenantId = cliCheck.tenantId;
         this._appId = cliCheck.azureLoginAppId;
         return result;

packages/databricks-vscode/src/configuration/auth/AzureCliCheck.ts

@@ -46,7 +46,7 @@ export class AzureCliCheck implements Disposable {
         this.disposables = [];
     }
 
-    public async check(silent = false): Promise<boolean> {
+    public async check(): Promise<boolean> {
         this.tenantId = this.authProvider.tenantId;
 
         let loginAttempts = 0;
@@ -156,11 +156,12 @@ export class AzureCliCheck implements Disposable {
                 message = e.message;
             }
 
+            NamedLogger.getOrCreate(Loggers.Extension).error(message, e);
             window.showErrorMessage(message);
             return false;
         }
 
-        if (result && !silent) {
+        if (result) {
             window.showInformationMessage(
                 "Databricks: Successfully logged in with Azure CLI"
             );

packages/databricks-vscode/src/configuration/auth/DatabricksCliCheck.ts

@@ -2,10 +2,12 @@ import {
     ExecUtils,
     ProductVersion,
     WorkspaceClient,
+    logging,
 } from "@databricks/databricks-sdk";
 import {Disposable, window} from "vscode";
 import {DatabricksCliAuthProvider} from "./AuthProvider";
 import {orchestrate, OrchestrationLoopError, Step} from "./orchestrate";
+import {Loggers} from "../../logger";
 
 // eslint-disable-next-line @typescript-eslint/no-var-requires
 const extensionVersion = require("../../../package.json")
@@ -23,7 +25,7 @@ export class DatabricksCliCheck implements Disposable {
         this.disposables = [];
     }
 
-    async check(silent: boolean): Promise<boolean> {
+    async check(): Promise<boolean> {
         const steps: Record<StepName, Step<boolean, StepName>> = {
             tryLogin: async () => {
                 if (await this.tryLogin()) {
@@ -55,12 +57,15 @@ export class DatabricksCliCheck implements Disposable {
             } else {
                 message = e.message;
             }
-
+            logging.NamedLogger.getOrCreate(Loggers.Extension).error(
+                message,
+                e
+            );
             window.showErrorMessage(message);
             return false;
         }
 
-        if (result && !silent) {
+        if (result) {
             window.showInformationMessage(
                 "Databricks: Successfully logged in with Databricks CLI"
             );

packages/databricks-vscode/src/configuration/auth/orchestrate.ts

@@ -42,7 +42,7 @@ export async function orchestrate<S, KEYS extends string>(
             throw new OrchestrationLoopError();
         }
         const result: StepResult<S, KEYS> = await steps[step]();
-        logger?.info(`Azire CLI check: ${step}`, result);
+        logger?.info(`Auth check: ${step}`, result);
 
         if (result.type === "error") {
             throw result.error;