Apply API recommendations for Create Env API (microsoft#21804)

Closes microsoft#21090

Author: karthiknadig

src/client/pythonEnvironments/creation/createEnvironment.ts

@@ -33,14 +33,12 @@ function fireStartedEvent(options?: CreateEnvironmentOptions): void {
 }
 
 function fireExitedEvent(result?: CreateEnvironmentResult, options?: CreateEnvironmentOptions, error?: Error): void {
- onCreateEnvironmentExitedEvent.fire({
- options,
- workspaceFolder: result?.workspaceFolder,
- path: result?.path,
- action: result?.action,
- error: error || result?.error,
- });
  startedEventCount -= 1;
+ if (result) {
+ onCreateEnvironmentExitedEvent.fire({ options, ...result });
+ } else if (error) {
+ onCreateEnvironmentExitedEvent.fire({ options, error });
+ }
 }
 
 export function getCreationEvents(): {
@@ -195,5 +193,8 @@ export async function handleCreateEnvironmentCommand(
  }
  }
 
- return result;
+ if (result) {
+ return Object.freeze(result);
+ }
+ return undefined;
 }

src/client/pythonEnvironments/creation/proposed.createEnvApis.ts

@@ -40,40 +40,83 @@ export interface EnvironmentWillCreateEvent {
  /**
  * Options used to create a Python environment.
  */
- options: CreateEnvironmentOptions | undefined;
+ readonly options: CreateEnvironmentOptions | undefined;
 }
 
+export type CreateEnvironmentResult =
+ | {
+ /**
+ * Workspace folder associated with the environment.
+ */
+ readonly workspaceFolder?: WorkspaceFolder;
+
+ /**
+ * Path to the executable python in the environment
+ */
+ readonly path: string;
+
+ /**
+ * User action that resulted in exit from the create environment flow.
+ */
+ readonly action?: CreateEnvironmentUserActions;
+
+ /**
+ * Error if any occurred during environment creation.
+ */
+ readonly error?: Error;
+ }
+ | {
+ /**
+ * Workspace folder associated with the environment.
+ */
+ readonly workspaceFolder?: WorkspaceFolder;
+
+ /**
+ * Path to the executable python in the environment
+ */
+ readonly path?: string;
+
+ /**
+ * User action that resulted in exit from the create environment flow.
+ */
+ readonly action: CreateEnvironmentUserActions;
+
+ /**
+ * Error if any occurred during environment creation.
+ */
+ readonly error?: Error;
+ }
+ | {
+ /**
+ * Workspace folder associated with the environment.
+ */
+ readonly workspaceFolder?: WorkspaceFolder;
+
+ /**
+ * Path to the executable python in the environment
+ */
+ readonly path?: string;
+
+ /**
+ * User action that resulted in exit from the create environment flow.
+ */
+ readonly action?: CreateEnvironmentUserActions;
+
+ /**
+ * Error if any occurred during environment creation.
+ */
+ readonly error: Error;
+ };
+
 /**
  * Params passed on `onDidCreateEnvironment` event handler.
  */
-export interface EnvironmentDidCreateEvent extends CreateEnvironmentResult {
+export type EnvironmentDidCreateEvent = CreateEnvironmentResult & {
  /**
  * Options used to create the Python environment.
  */
- options: CreateEnvironmentOptions | undefined;
-}
-
-export interface CreateEnvironmentResult {
- /**
- * Workspace folder associated with the environment.
- */
- workspaceFolder: WorkspaceFolder | undefined;
-
- /**
- * Path to the executable python in the environment
- */
- path: string | undefined;
-
- /**
- * User action that resulted in exit from the create environment flow.
- */
- action: CreateEnvironmentUserActions | undefined;
-
- /**
- * Error if any occurred during environment creation.
- */
- error: Error | undefined;
-}
+ readonly options: CreateEnvironmentOptions | undefined;
+};
 
 /**
  * Extensions that want to contribute their own environment creation can do that by registering an object
@@ -120,14 +163,14 @@ export interface ProposedCreateEnvironmentAPI {
  * provider (including internal providers). This will also receive any options passed in
  * or defaults used to create environment.
  */
- onWillCreateEnvironment: Event<EnvironmentWillCreateEvent>;
+ readonly onWillCreateEnvironment: Event<EnvironmentWillCreateEvent>;
 
  /**
  * This API can be used to detect when the environment provider exits for any registered
  * provider (including internal providers). This will also receive created environment path,
  * any errors, or user actions taken from the provider.
  */
- onDidCreateEnvironment: Event<EnvironmentDidCreateEvent>;
+ readonly onDidCreateEnvironment: Event<EnvironmentDidCreateEvent>;
 
  /**
  * This API will show a QuickPick to select an environment provider from available list of

src/client/pythonEnvironments/creation/provider/condaCreationProvider.ts

@@ -4,7 +4,7 @@
 import { CancellationToken, ProgressLocation, WorkspaceFolder } from 'vscode';
 import * as path from 'path';
 import { Commands, PVSC_EXTENSION_ID } from '../../../common/constants';
-import { traceError, traceLog } from '../../../logging';
+import { traceError, traceInfo, traceLog } from '../../../logging';
 import { CreateEnvironmentProgress } from '../types';
 import { pickWorkspaceFolder } from '../common/workspaceSelection';
 import { execObservable } from '../../../common/process/rawProcessApis';
@@ -77,7 +77,7 @@ async function createCondaEnv(
  args: string[],
  progress: CreateEnvironmentProgress,
  token?: CancellationToken,
-): Promise<string | undefined> {
+): Promise<string> {
  progress.report({
  message: CreateEnv.Conda.creating,
  });
@@ -174,6 +174,7 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
  traceError('Workspace was not selected or found for creating conda environment.');
  return MultiStepAction.Cancel;
  }
+ traceInfo(`Selected workspace ${workspace.uri.fsPath} for creating conda environment.`);
  return MultiStepAction.Continue;
  },
  undefined,
@@ -196,6 +197,7 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
  traceError('Python version was not selected for creating conda environment.');
  return MultiStepAction.Cancel;
  }
+ traceInfo(`Selected Python version ${version} for creating conda environment.`);
  return MultiStepAction.Continue;
  },
  undefined,
@@ -217,8 +219,6 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
  progress: CreateEnvironmentProgress,
  token: CancellationToken,
  ): Promise<CreateEnvironmentResult | undefined> => {
- let hasError = false;
-
  progress.report({
  message: CreateEnv.statusStarting,
  });
@@ -237,17 +237,20 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
  progress,
  token,
  );
+
+ if (envPath) {
+ return { path: envPath, workspaceFolder: workspace };
+ }
+
+ throw new Error('Failed to create conda environment. See Output > Python for more info.');
+ } else {
+ throw new Error('A workspace is needed to create conda environment');
  }
  } catch (ex) {
  traceError(ex);
- hasError = true;
+ showErrorMessageWithLogs(CreateEnv.Conda.errorCreatingEnvironment);
  throw ex;
- } finally {
- if (hasError) {
- showErrorMessageWithLogs(CreateEnv.Conda.errorCreatingEnvironment);
- }
  }
- return { path: envPath, workspaceFolder: workspace, action: undefined, error: undefined };
  },
  );
 }

src/client/pythonEnvironments/creation/provider/venvCreationProvider.ts

@@ -8,7 +8,7 @@ import { createVenvScript } from '../../../common/process/internal/scripts';
 import { execObservable } from '../../../common/process/rawProcessApis';
 import { createDeferred } from '../../../common/utils/async';
 import { Common, CreateEnv } from '../../../common/utils/localize';
-import { traceError, traceLog, traceVerbose } from '../../../logging';
+import { traceError, traceInfo, traceLog, traceVerbose } from '../../../logging';
 import { CreateEnvironmentProgress } from '../types';
 import { pickWorkspaceFolder } from '../common/workspaceSelection';
 import { IInterpreterQuickPick } from '../../../interpreter/configuration/types';
@@ -144,6 +144,7 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
  traceError('Workspace was not selected or found for creating virtual environment.');
  return MultiStepAction.Cancel;
  }
+ traceInfo(`Selected workspace ${workspace.uri.fsPath} for creating virtual environment.`);
  return MultiStepAction.Continue;
  },
  undefined,
@@ -183,6 +184,7 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
  traceError('Virtual env creation requires an interpreter.');
  return MultiStepAction.Cancel;
  }
+ traceInfo(`Selected interpreter ${interpreter} for creating virtual environment.`);
  return MultiStepAction.Continue;
  },
  undefined,
@@ -237,8 +239,6 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
  progress: CreateEnvironmentProgress,
  token: CancellationToken,
  ): Promise<CreateEnvironmentResult | undefined> => {
- let hasError = false;
-
  progress.report({
  message: CreateEnv.statusStarting,
  });
@@ -247,18 +247,19 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
  try {
  if (interpreter && workspace) {
  envPath = await createVenv(workspace, interpreter, args, progress, token);
+ if (envPath) {
+ return { path: envPath, workspaceFolder: workspace };
+ }
+ throw new Error('Failed to create virtual environment. See Output > Python for more info.');
  }
+ throw new Error(
+ 'Failed to create virtual environment. Either interpreter or workspace is undefined.',
+ );
  } catch (ex) {
  traceError(ex);
- hasError = true;
+ showErrorMessageWithLogs(CreateEnv.Venv.errorCreatingEnvironment);
  throw ex;
- } finally {
- if (hasError) {
- showErrorMessageWithLogs(CreateEnv.Venv.errorCreatingEnvironment);
- }
  }
-
- return { path: envPath, workspaceFolder: workspace, action: undefined, error: undefined };
  },
  );
  }

src/test/pythonEnvironments/creation/provider/condaCreationProvider.unit.test.ts

@@ -134,8 +134,6 @@ suite('Conda Creation provider tests', () => {
  assert.deepStrictEqual(await promise, {
  path: 'new_environment',
  workspaceFolder: workspace1,
- action: undefined,
- error: undefined,
  });
  assert.isTrue(showErrorMessageWithLogsStub.notCalled);
  });

src/test/pythonEnvironments/creation/provider/venvCreationProvider.unit.test.ts

@@ -158,8 +158,6 @@ suite('venv Creation provider tests', () => {
  assert.deepStrictEqual(actual, {
  path: 'new_environment',
  workspaceFolder: workspace1,
- action: undefined,
- error: undefined,
  });
  interpreterQuickPick.verifyAll();
  progressMock.verifyAll();