Skip to content

Commit 5140a8d

Browse files
karthiknadigkarthiknadig
authored
Apply API recommendations for Create Env API (#21804)
Closes #21090
1 parent 0248fa8 commit 5140a8d

File tree

6 files changed

+104
-60
lines changed

6 files changed

+104
-60
lines changed

‎src/client/pythonEnvironments/creation/createEnvironment.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,12 @@ function fireStartedEvent(options?: CreateEnvironmentOptions): void {
3333
}
3434

3535
function fireExitedEvent(result?: CreateEnvironmentResult, options?: CreateEnvironmentOptions, error?: Error): void {
36-
onCreateEnvironmentExitedEvent.fire({
37-
options,
38-
workspaceFolder: result?.workspaceFolder,
39-
path: result?.path,
40-
action: result?.action,
41-
error: error || result?.error,
42-
});
4336
startedEventCount -= 1;
37+
if (result) {
38+
onCreateEnvironmentExitedEvent.fire({ options, ...result });
39+
} else if (error) {
40+
onCreateEnvironmentExitedEvent.fire({ options, error });
41+
}
4442
}
4543

4644
export function getCreationEvents(): {
@@ -195,5 +193,8 @@ export async function handleCreateEnvironmentCommand(
195193
}
196194
}
197195

198-
return result;
196+
if (result) {
197+
return Object.freeze(result);
198+
}
199+
return undefined;
199200
}

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

Lines changed: 71 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -40,40 +40,83 @@ export interface EnvironmentWillCreateEvent {
4040
/**
4141
* Options used to create a Python environment.
4242
*/
43-
options: CreateEnvironmentOptions | undefined;
43+
readonly options: CreateEnvironmentOptions | undefined;
4444
}
4545

46+
export type CreateEnvironmentResult =
47+
| {
48+
/**
49+
* Workspace folder associated with the environment.
50+
*/
51+
readonly workspaceFolder?: WorkspaceFolder;
52+
53+
/**
54+
* Path to the executable python in the environment
55+
*/
56+
readonly path: string;
57+
58+
/**
59+
* User action that resulted in exit from the create environment flow.
60+
*/
61+
readonly action?: CreateEnvironmentUserActions;
62+
63+
/**
64+
* Error if any occurred during environment creation.
65+
*/
66+
readonly error?: Error;
67+
}
68+
| {
69+
/**
70+
* Workspace folder associated with the environment.
71+
*/
72+
readonly workspaceFolder?: WorkspaceFolder;
73+
74+
/**
75+
* Path to the executable python in the environment
76+
*/
77+
readonly path?: string;
78+
79+
/**
80+
* User action that resulted in exit from the create environment flow.
81+
*/
82+
readonly action: CreateEnvironmentUserActions;
83+
84+
/**
85+
* Error if any occurred during environment creation.
86+
*/
87+
readonly error?: Error;
88+
}
89+
| {
90+
/**
91+
* Workspace folder associated with the environment.
92+
*/
93+
readonly workspaceFolder?: WorkspaceFolder;
94+
95+
/**
96+
* Path to the executable python in the environment
97+
*/
98+
readonly path?: string;
99+
100+
/**
101+
* User action that resulted in exit from the create environment flow.
102+
*/
103+
readonly action?: CreateEnvironmentUserActions;
104+
105+
/**
106+
* Error if any occurred during environment creation.
107+
*/
108+
readonly error: Error;
109+
};
110+
46111
/**
47112
* Params passed on `onDidCreateEnvironment` event handler.
48113
*/
49-
export interface EnvironmentDidCreateEvent extends CreateEnvironmentResult {
114+
export type EnvironmentDidCreateEvent = CreateEnvironmentResult & {
50115
/**
51116
* Options used to create the Python environment.
52117
*/
53-
options: CreateEnvironmentOptions | undefined;
54-
}
55-
56-
export interface CreateEnvironmentResult {
57-
/**
58-
* Workspace folder associated with the environment.
59-
*/
60-
workspaceFolder: WorkspaceFolder | undefined;
61-
62-
/**
63-
* Path to the executable python in the environment
64-
*/
65-
path: string | undefined;
66-
67-
/**
68-
* User action that resulted in exit from the create environment flow.
69-
*/
70-
action: CreateEnvironmentUserActions | undefined;
71-
72-
/**
73-
* Error if any occurred during environment creation.
74-
*/
75-
error: Error | undefined;
76-
}
118+
readonly options: CreateEnvironmentOptions | undefined;
119+
};
77120

78121
/**
79122
* Extensions that want to contribute their own environment creation can do that by registering an object
@@ -120,14 +163,14 @@ export interface ProposedCreateEnvironmentAPI {
120163
* provider (including internal providers). This will also receive any options passed in
121164
* or defaults used to create environment.
122165
*/
123-
onWillCreateEnvironment: Event<EnvironmentWillCreateEvent>;
166+
readonly onWillCreateEnvironment: Event<EnvironmentWillCreateEvent>;
124167

125168
/**
126169
* This API can be used to detect when the environment provider exits for any registered
127170
* provider (including internal providers). This will also receive created environment path,
128171
* any errors, or user actions taken from the provider.
129172
*/
130-
onDidCreateEnvironment: Event<EnvironmentDidCreateEvent>;
173+
readonly onDidCreateEnvironment: Event<EnvironmentDidCreateEvent>;
131174

132175
/**
133176
* This API will show a QuickPick to select an environment provider from available list of

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import { CancellationToken, ProgressLocation, WorkspaceFolder } from 'vscode';
55
import * as path from 'path';
66
import { Commands, PVSC_EXTENSION_ID } from '../../../common/constants';
7-
import { traceError, traceLog } from '../../../logging';
7+
import { traceError, traceInfo, traceLog } from '../../../logging';
88
import { CreateEnvironmentProgress } from '../types';
99
import { pickWorkspaceFolder } from '../common/workspaceSelection';
1010
import { execObservable } from '../../../common/process/rawProcessApis';
@@ -77,7 +77,7 @@ async function createCondaEnv(
7777
args: string[],
7878
progress: CreateEnvironmentProgress,
7979
token?: CancellationToken,
80-
): Promise<string | undefined> {
80+
): Promise<string> {
8181
progress.report({
8282
message: CreateEnv.Conda.creating,
8383
});
@@ -174,6 +174,7 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
174174
traceError('Workspace was not selected or found for creating conda environment.');
175175
return MultiStepAction.Cancel;
176176
}
177+
traceInfo(`Selected workspace ${workspace.uri.fsPath} for creating conda environment.`);
177178
return MultiStepAction.Continue;
178179
},
179180
undefined,
@@ -196,6 +197,7 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
196197
traceError('Python version was not selected for creating conda environment.');
197198
return MultiStepAction.Cancel;
198199
}
200+
traceInfo(`Selected Python version ${version} for creating conda environment.`);
199201
return MultiStepAction.Continue;
200202
},
201203
undefined,
@@ -217,8 +219,6 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
217219
progress: CreateEnvironmentProgress,
218220
token: CancellationToken,
219221
): Promise<CreateEnvironmentResult | undefined> => {
220-
let hasError = false;
221-
222222
progress.report({
223223
message: CreateEnv.statusStarting,
224224
});
@@ -237,17 +237,20 @@ async function createEnvironment(options?: CreateEnvironmentOptions): Promise<Cr
237237
progress,
238238
token,
239239
);
240+
241+
if (envPath) {
242+
return { path: envPath, workspaceFolder: workspace };
243+
}
244+
245+
throw new Error('Failed to create conda environment. See Output > Python for more info.');
246+
} else {
247+
throw new Error('A workspace is needed to create conda environment');
240248
}
241249
} catch (ex) {
242250
traceError(ex);
243-
hasError = true;
251+
showErrorMessageWithLogs(CreateEnv.Conda.errorCreatingEnvironment);
244252
throw ex;
245-
} finally {
246-
if (hasError) {
247-
showErrorMessageWithLogs(CreateEnv.Conda.errorCreatingEnvironment);
248-
}
249253
}
250-
return { path: envPath, workspaceFolder: workspace, action: undefined, error: undefined };
251254
},
252255
);
253256
}

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

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { createVenvScript } from '../../../common/process/internal/scripts';
88
import { execObservable } from '../../../common/process/rawProcessApis';
99
import { createDeferred } from '../../../common/utils/async';
1010
import { Common, CreateEnv } from '../../../common/utils/localize';
11-
import { traceError, traceLog, traceVerbose } from '../../../logging';
11+
import { traceError, traceInfo, traceLog, traceVerbose } from '../../../logging';
1212
import { CreateEnvironmentProgress } from '../types';
1313
import { pickWorkspaceFolder } from '../common/workspaceSelection';
1414
import { IInterpreterQuickPick } from '../../../interpreter/configuration/types';
@@ -144,6 +144,7 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
144144
traceError('Workspace was not selected or found for creating virtual environment.');
145145
return MultiStepAction.Cancel;
146146
}
147+
traceInfo(`Selected workspace ${workspace.uri.fsPath} for creating virtual environment.`);
147148
return MultiStepAction.Continue;
148149
},
149150
undefined,
@@ -183,6 +184,7 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
183184
traceError('Virtual env creation requires an interpreter.');
184185
return MultiStepAction.Cancel;
185186
}
187+
traceInfo(`Selected interpreter ${interpreter} for creating virtual environment.`);
186188
return MultiStepAction.Continue;
187189
},
188190
undefined,
@@ -237,8 +239,6 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
237239
progress: CreateEnvironmentProgress,
238240
token: CancellationToken,
239241
): Promise<CreateEnvironmentResult | undefined> => {
240-
let hasError = false;
241-
242242
progress.report({
243243
message: CreateEnv.statusStarting,
244244
});
@@ -247,18 +247,19 @@ export class VenvCreationProvider implements CreateEnvironmentProvider {
247247
try {
248248
if (interpreter && workspace) {
249249
envPath = await createVenv(workspace, interpreter, args, progress, token);
250+
if (envPath) {
251+
return { path: envPath, workspaceFolder: workspace };
252+
}
253+
throw new Error('Failed to create virtual environment. See Output > Python for more info.');
250254
}
255+
throw new Error(
256+
'Failed to create virtual environment. Either interpreter or workspace is undefined.',
257+
);
251258
} catch (ex) {
252259
traceError(ex);
253-
hasError = true;
260+
showErrorMessageWithLogs(CreateEnv.Venv.errorCreatingEnvironment);
254261
throw ex;
255-
} finally {
256-
if (hasError) {
257-
showErrorMessageWithLogs(CreateEnv.Venv.errorCreatingEnvironment);
258-
}
259262
}
260-
261-
return { path: envPath, workspaceFolder: workspace, action: undefined, error: undefined };
262263
},
263264
);
264265
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,6 @@ suite('Conda Creation provider tests', () => {
134134
assert.deepStrictEqual(await promise, {
135135
path: 'new_environment',
136136
workspaceFolder: workspace1,
137-
action: undefined,
138-
error: undefined,
139137
});
140138
assert.isTrue(showErrorMessageWithLogsStub.notCalled);
141139
});

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,6 @@ suite('venv Creation provider tests', () => {
158158
assert.deepStrictEqual(actual, {
159159
path: 'new_environment',
160160
workspaceFolder: workspace1,
161-
action: undefined,
162-
error: undefined,
163161
});
164162
interpreterQuickPick.verifyAll();
165163
progressMock.verifyAll();

0 commit comments

Comments
 (0)