Skip to content

Commit d30b660

Browse files
Kartik Rajpaulacamargo25
Kartik Raj
and
paulacamargo25
authored
Update python execution factory using conda.ts (#18136)
* Add conda run command * Fix code run * Fix linting * Fix tests * Add news * Fix tests in linter * Fix strings * Fix linters * Fix conda run unit test * Fix funtional tests * Fix single workspace tests * update minimum conda versionvalue * Change sorting timeput * Add timeout time in sorting test * Fix test wothout config * Add more time to timeout in sorting tests * Add conda run command * Fix code run * Fix linting * Fix tests * Add news * Fix tests in linter * Fix strings * Fix linters * Fix conda run unit test * Fix funtional tests * Fix single workspace tests * update minimum conda versionvalue * Change sorting timeput * Add timeout time in sorting test * Add more time to timeout in sorting tests * Update news/1 Enhancements/7696.md Co-authored-by: Kartik Raj <karraj@microsoft.com> * Rix pylint test * Fix lint test * Remove unnecessary interpreter check * Use conda.ts * Fix tests * Remove unnecessary export * Rebase * Mege * Fix tests Co-authored-by: Paula Camargo <paulitacv25@gmail.com>
1 parent 0d815df commit d30b660

9 files changed

Lines changed: 123 additions & 178 deletions

File tree

‎src/client/common/process/pythonEnvironment.ts‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,16 @@
22
// Licensed under the MIT License.
33

44
import { traceError, traceInfo } from '../../logging';
5-
import { CondaEnvironmentInfo } from '../../pythonEnvironments/common/environmentManagers/conda';
5+
import { Conda, CondaEnvironmentInfo } from '../../pythonEnvironments/common/environmentManagers/conda';
66
import { buildPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/exec';
77
import { InterpreterInformation } from '../../pythonEnvironments/info';
88
import { getExecutablePath } from '../../pythonEnvironments/info/executable';
99
import { getInterpreterInfo } from '../../pythonEnvironments/info/interpreter';
1010
import { IFileSystem } from '../platform/types';
1111
import * as internalPython from './internal/python';
12-
import { ExecutionResult, IProcessService, ShellOptions, SpawnOptions } from './types';
12+
import { ExecutionResult, IProcessService, IPythonEnvironment, ShellOptions, SpawnOptions } from './types';
1313

14-
class PythonEnvironment {
14+
class PythonEnvironment implements IPythonEnvironment {
1515
private cachedExecutablePath: Map<string, Promise<string>> = new Map<string, Promise<string>>();
1616
private cachedInterpreterInformation: InterpreterInformation | undefined | null = null;
1717

@@ -28,13 +28,13 @@ class PythonEnvironment {
2828
},
2929
) {}
3030

31-
public getExecutionInfo(pythonArgs: string[] = []): PythonExecInfo {
31+
public getExecutionInfo(pythonArgs: string[] = [], pythonExecutable?: string): PythonExecInfo {
3232
const python = this.deps.getPythonArgv(this.pythonPath);
33-
return buildPythonExecInfo(python, pythonArgs);
33+
return buildPythonExecInfo(python, pythonArgs, pythonExecutable);
3434
}
35-
public getExecutionObservableInfo(pythonArgs: string[] = []): PythonExecInfo {
35+
public getExecutionObservableInfo(pythonArgs: string[] = [], pythonExecutable?: string): PythonExecInfo {
3636
const python = this.deps.getObservablePythonArgv(this.pythonPath);
37-
return buildPythonExecInfo(python, pythonArgs);
37+
return buildPythonExecInfo(python, pythonArgs, pythonExecutable);
3838
}
3939

4040
public async getInterpreterInformation(): Promise<InterpreterInformation | undefined> {
@@ -131,21 +131,18 @@ export function createPythonEnv(
131131
return new PythonEnvironment(pythonPath, deps);
132132
}
133133

134-
export function createCondaEnv(
135-
condaFile: string,
134+
export async function createCondaEnv(
136135
condaInfo: CondaEnvironmentInfo,
137136
pythonPath: string,
138137
// These are used to generate the deps.
139138
procs: IProcessService,
140139
fs: IFileSystem,
141-
): PythonEnvironment {
142-
const runArgs = ['run'];
143-
if (condaInfo.name === '') {
144-
runArgs.push('-p', condaInfo.path);
145-
} else {
146-
runArgs.push('-n', condaInfo.name);
140+
): Promise<PythonEnvironment | undefined> {
141+
const conda = await Conda.getConda();
142+
const pythonArgv = await conda?.getRunPythonArgs({ name: condaInfo.name, prefix: condaInfo.path });
143+
if (!pythonArgv) {
144+
return undefined;
147145
}
148-
const pythonArgv = [condaFile, ...runArgs, '--no-capture-output', 'python'];
149146
const deps = createDeps(
150147
async (filename) => fs.pathExists(filename),
151148
pythonArgv,

‎src/client/common/process/pythonExecutionFactory.ts‎

Lines changed: 19 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33
import { inject, injectable } from 'inversify';
4-
import { gte } from 'semver';
54

6-
import { Uri } from 'vscode';
75
import { IEnvironmentActivationService } from '../../interpreter/activation/types';
8-
import { IComponentAdapter, ICondaService } from '../../interpreter/contracts';
6+
import { IComponentAdapter } from '../../interpreter/contracts';
97
import { IServiceContainer } from '../../ioc/types';
10-
import { CondaEnvironmentInfo } from '../../pythonEnvironments/common/environmentManagers/conda';
118
import { sendTelemetryEvent } from '../../telemetry';
129
import { EventName } from '../../telemetry/constants';
1310
import { IFileSystem } from '../platform/types';
@@ -22,16 +19,14 @@ import {
2219
IProcessLogger,
2320
IProcessService,
2421
IProcessServiceFactory,
22+
IPythonEnvironment,
2523
IPythonExecutionFactory,
2624
IPythonExecutionService,
2725
} from './types';
2826
import { IInterpreterAutoSelectionService } from '../../interpreter/autoSelection/types';
2927
import { sleep } from '../utils/async';
3028
import { traceError } from '../../logging';
3129

32-
// Minimum version number of conda required to be able to use 'conda run' and '--no-capture--output' option
33-
export const CONDA_RUN_VERSION = '4.9.0';
34-
3530
@injectable()
3631
export class PythonExecutionFactory implements IPythonExecutionFactory {
3732
private readonly disposables: IDisposableRegistry;
@@ -45,7 +40,6 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
4540
@inject(IEnvironmentActivationService) private readonly activationHelper: IEnvironmentActivationService,
4641
@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
4742
@inject(IConfigurationService) private readonly configService: IConfigurationService,
48-
@inject(ICondaService) private readonly condaService: ICondaService,
4943
@inject(IBufferDecoder) private readonly decoder: IBufferDecoder,
5044
@inject(IComponentAdapter) private readonly pyenvs: IComponentAdapter,
5145
@inject(IInterpreterAutoSelectionService) private readonly autoSelection: IInterpreterAutoSelectionService,
@@ -90,13 +84,11 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
9084

9185
const windowsStoreInterpreterCheck = this.pyenvs.isWindowsStoreInterpreter.bind(this.pyenvs);
9286

93-
return createPythonService(
94-
pythonPath,
95-
processService,
96-
this.fileSystem,
97-
undefined,
98-
await windowsStoreInterpreterCheck(pythonPath),
99-
);
87+
const env = (await windowsStoreInterpreterCheck(pythonPath))
88+
? createWindowsStoreEnv(pythonPath, processService)
89+
: createPythonEnv(pythonPath, processService, this.fileSystem);
90+
91+
return createPythonService(processService, env);
10092
}
10193

10294
public async createActivatedEnvironment(
@@ -126,60 +118,28 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
126118
if (condaExecutionService) {
127119
return condaExecutionService;
128120
}
129-
130-
return createPythonService(pythonPath, processService, this.fileSystem);
121+
const env = createPythonEnv(pythonPath, processService, this.fileSystem);
122+
return createPythonService(processService, env);
131123
}
132124

133125
public async createCondaExecutionService(
134126
pythonPath: string,
135-
processService?: IProcessService,
136-
resource?: Uri,
127+
processService: IProcessService,
137128
): Promise<IPythonExecutionService | undefined> {
138-
const processServicePromise = processService
139-
? Promise.resolve(processService)
140-
: this.processServiceFactory.create(resource);
141129
const condaLocatorService = this.serviceContainer.get<IComponentAdapter>(IComponentAdapter);
142-
const [condaVersion, condaEnvironment, condaFile, procService] = await Promise.all([
143-
this.condaService.getCondaVersion(),
144-
condaLocatorService.getCondaEnvironment(pythonPath),
145-
this.condaService.getCondaFile(),
146-
processServicePromise,
147-
]);
148-
149-
if (condaVersion && gte(condaVersion, CONDA_RUN_VERSION) && condaEnvironment && condaFile && procService) {
150-
// Add logging to the newly created process service
151-
if (!processService) {
152-
procService.on('exec', this.logger.logProcess.bind(this.logger));
153-
this.disposables.push(procService);
154-
}
155-
156-
return createPythonService(
157-
pythonPath,
158-
procService,
159-
this.fileSystem,
160-
// This is what causes a CondaEnvironment to be returned:
161-
[condaFile, condaEnvironment],
162-
);
130+
const [condaEnvironment] = await Promise.all([condaLocatorService.getCondaEnvironment(pythonPath)]);
131+
if (!condaEnvironment) {
132+
return undefined;
163133
}
164-
165-
return Promise.resolve(undefined);
134+
const env = await createCondaEnv(condaEnvironment, pythonPath, processService, this.fileSystem);
135+
if (!env) {
136+
return undefined;
137+
}
138+
return createPythonService(processService, env);
166139
}
167140
}
168141

169-
function createPythonService(
170-
pythonPath: string,
171-
procService: IProcessService,
172-
fs: IFileSystem,
173-
conda?: [string, CondaEnvironmentInfo],
174-
isWindowsStore?: boolean,
175-
): IPythonExecutionService {
176-
let env = createPythonEnv(pythonPath, procService, fs);
177-
if (conda) {
178-
const [condaPath, condaInfo] = conda;
179-
env = createCondaEnv(condaPath, condaInfo, pythonPath, procService, fs);
180-
} else if (isWindowsStore) {
181-
env = createWindowsStoreEnv(pythonPath, procService);
182-
}
142+
function createPythonService(procService: IProcessService, env: IPythonEnvironment): IPythonExecutionService {
183143
const procs = createPythonProcessService(procService, env);
184144
return {
185145
getInterpreterInformation: () => env.getInterpreterInformation(),

‎src/client/common/process/pythonProcess.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { PythonExecInfo } from '../../pythonEnvironments/exec';
55
import { ErrorUtils } from '../errors/errorUtils';
66
import { ModuleNotInstalledError } from '../errors/moduleNotInstalledError';
77
import * as internalPython from './internal/python';
8-
import { ExecutionResult, IProcessService, ObservableExecutionResult, SpawnOptions } from './types';
8+
import { ExecutionResult, IProcessService, IPythonEnvironment, ObservableExecutionResult, SpawnOptions } from './types';
99

1010
class PythonProcessService {
1111
constructor(
@@ -89,11 +89,7 @@ class PythonProcessService {
8989
export function createPythonProcessService(
9090
procs: IProcessService,
9191
// from PythonEnvironment:
92-
env: {
93-
getExecutionInfo(pythonArgs?: string[]): PythonExecInfo;
94-
getExecutionObservableInfo(pythonArgs?: string[]): PythonExecInfo;
95-
isModuleInstalled(moduleName: string): Promise<boolean>;
96-
},
92+
env: IPythonEnvironment,
9793
) {
9894
const deps = {
9995
// from PythonService:

‎src/client/common/process/types.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,7 @@ export interface IPythonExecutionFactory {
8282
createActivatedEnvironment(options: ExecutionFactoryCreateWithEnvironmentOptions): Promise<IPythonExecutionService>;
8383
createCondaExecutionService(
8484
pythonPath: string,
85-
processService?: IProcessService,
86-
resource?: Uri,
85+
processService: IProcessService,
8786
): Promise<IPythonExecutionService | undefined>;
8887
}
8988
export const IPythonExecutionService = Symbol('IPythonExecutionService');
@@ -103,6 +102,15 @@ export interface IPythonExecutionService {
103102
execForLinter(moduleName: string, args: string[], options: SpawnOptions): Promise<ExecutionResult<string>>;
104103
}
105104

105+
export interface IPythonEnvironment {
106+
getInterpreterInformation(): Promise<InterpreterInformation | undefined>;
107+
getExecutionObservableInfo(pythonArgs?: string[], pythonExecutable?: string): PythonExecInfo;
108+
getExecutablePath(): Promise<string>;
109+
isModuleInstalled(moduleName: string): Promise<boolean>;
110+
getModuleVersion(moduleName: string): Promise<string | undefined>;
111+
getExecutionInfo(pythonArgs?: string[], pythonExecutable?: string): PythonExecInfo;
112+
}
113+
106114
export class StdErrError extends Error {
107115
constructor(message: string) {
108116
super(message);

‎src/test/common/process/pythonEnvironment.unit.test.ts‎

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import { expect, use } from 'chai';
55
import * as chaiAsPromised from 'chai-as-promised';
6+
import * as sinon from 'sinon';
67
import { SemVer } from 'semver';
78
import * as TypeMoq from 'typemoq';
89
import { IFileSystem } from '../../../client/common/platform/types';
@@ -13,6 +14,8 @@ import {
1314
} from '../../../client/common/process/pythonEnvironment';
1415
import { IProcessService, StdErrError } from '../../../client/common/process/types';
1516
import { Architecture } from '../../../client/common/utils/platform';
17+
import { Conda } from '../../../client/pythonEnvironments/common/environmentManagers/conda';
18+
import { OUTPUT_MARKER_SCRIPT } from '../../../client/common/process/internal/scripts';
1619

1720
use(chaiAsPromised);
1821

@@ -275,64 +278,67 @@ suite('CondaEnvironment', () => {
275278
const condaFile = 'path/to/conda';
276279

277280
setup(() => {
281+
sinon.stub(Conda, 'getConda').resolves(new Conda(condaFile));
278282
processService = TypeMoq.Mock.ofType<IProcessService>(undefined, TypeMoq.MockBehavior.Strict);
279283
fileSystem = TypeMoq.Mock.ofType<IFileSystem>(undefined, TypeMoq.MockBehavior.Strict);
280284
});
281285

282-
test('getExecutionInfo with a named environment should return execution info using the environment name', () => {
286+
teardown(() => sinon.restore());
287+
288+
test('getExecutionInfo with a named environment should return execution info using the environment name', async () => {
283289
const condaInfo = { name: 'foo', path: 'bar' };
284-
const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
290+
const env = await createCondaEnv(condaInfo, pythonPath, processService.object, fileSystem.object);
285291

286-
const result = env.getExecutionInfo(args);
292+
const result = env?.getExecutionInfo(args, pythonPath);
287293

288294
expect(result).to.deep.equal({
289295
command: condaFile,
290-
args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', ...args],
291-
python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python'],
292-
pythonExecutable: 'python',
296+
args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT, ...args],
297+
python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT],
298+
pythonExecutable: pythonPath,
293299
});
294300
});
295301

296-
test('getExecutionInfo with a non-named environment should return execution info using the environment path', () => {
302+
test('getExecutionInfo with a non-named environment should return execution info using the environment path', async () => {
297303
const condaInfo = { name: '', path: 'bar' };
298-
const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
304+
const env = await createCondaEnv(condaInfo, pythonPath, processService.object, fileSystem.object);
299305

300-
const result = env.getExecutionInfo(args);
306+
const result = env?.getExecutionInfo(args, pythonPath);
301307

302308
expect(result).to.deep.equal({
303309
command: condaFile,
304-
args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', ...args],
305-
python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python'],
306-
pythonExecutable: 'python',
310+
args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT, ...args],
311+
python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT],
312+
pythonExecutable: pythonPath,
307313
});
308314
});
309315

310-
test('getExecutionObservableInfo with a named environment should return execution info using conda full path with the name', () => {
316+
test('getExecutionObservableInfo with a named environment should return execution info using conda full path with the name', async () => {
311317
const condaInfo = { name: 'foo', path: 'bar' };
312318
const expected = {
313319
command: condaFile,
314-
args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', ...args],
315-
python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python'],
316-
pythonExecutable: 'python',
320+
args: ['run', '-n', condaInfo.name, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT, ...args],
321+
python: [condaFile, 'run', '-n', condaInfo.name, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT],
322+
pythonExecutable: pythonPath,
317323
};
318-
const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
324+
const env = await createCondaEnv(condaInfo, pythonPath, processService.object, fileSystem.object);
319325

320-
const result = env.getExecutionObservableInfo(args);
326+
const result = env?.getExecutionObservableInfo(args, pythonPath);
321327

322328
expect(result).to.deep.equal(expected);
323329
});
324330

325-
test('getExecutionObservableInfo with a non-named environment should return execution info using conda full path', () => {
331+
test('getExecutionObservableInfo with a non-named environment should return execution info using conda full path', async () => {
326332
const condaInfo = { name: '', path: 'bar' };
327333
const expected = {
328334
command: condaFile,
329-
args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', ...args],
330-
python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python'],
331-
pythonExecutable: 'python',
335+
args: ['run', '-p', condaInfo.path, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT, ...args],
336+
python: [condaFile, 'run', '-p', condaInfo.path, '--no-capture-output', 'python', OUTPUT_MARKER_SCRIPT],
337+
pythonExecutable: pythonPath,
332338
};
333-
const env = createCondaEnv(condaFile, condaInfo, pythonPath, processService.object, fileSystem.object);
339+
const env = await createCondaEnv(condaInfo, pythonPath, processService.object, fileSystem.object);
334340

335-
const result = env.getExecutionObservableInfo(args);
341+
const result = env?.getExecutionObservableInfo(args, pythonPath);
336342

337343
expect(result).to.deep.equal(expected);
338344
});

0 commit comments

Comments
 (0)