Skip to content

Use conda run for conda environments for running python files #18520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/1 Enhancements/18479.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use `conda run` for conda environments for running python files and installing modules.
6 changes: 5 additions & 1 deletion src/client/common/process/pythonEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,13 @@ export async function createCondaEnv(
// These are used to generate the deps.
procs: IProcessService,
fs: IFileSystem,
executeAsAProcess?: boolean,
): Promise<PythonEnvironment | undefined> {
const conda = await Conda.getConda();
const pythonArgv = await conda?.getRunPythonArgs({ name: condaInfo.name, prefix: condaInfo.path });
const pythonArgv = await conda?.getRunPythonArgs(
{ name: condaInfo.name, prefix: condaInfo.path },
executeAsAProcess,
);
if (!pythonArgv) {
return undefined;
}
Expand Down
15 changes: 13 additions & 2 deletions src/client/common/process/pythonExecutionFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
}
const processService: IProcessService = await this.processServiceFactory.create(options.resource);

const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService);
const condaExecutionService = await this.createCondaExecutionService(
pythonPath,
processService,
options.executeAsAProcess,
);
if (condaExecutionService) {
return condaExecutionService;
}
Expand Down Expand Up @@ -125,13 +129,20 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
public async createCondaExecutionService(
pythonPath: string,
processService: IProcessService,
executeAsAProcess?: boolean,
): Promise<IPythonExecutionService | undefined> {
const condaLocatorService = this.serviceContainer.get<IComponentAdapter>(IComponentAdapter);
const [condaEnvironment] = await Promise.all([condaLocatorService.getCondaEnvironment(pythonPath)]);
if (!condaEnvironment) {
return undefined;
}
const env = await createCondaEnv(condaEnvironment, pythonPath, processService, this.fileSystem);
const env = await createCondaEnv(
condaEnvironment,
pythonPath,
processService,
this.fileSystem,
executeAsAProcess,
);
if (!env) {
return undefined;
}
Expand Down
4 changes: 4 additions & 0 deletions src/client/common/process/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ export const IPythonExecutionFactory = Symbol('IPythonExecutionFactory');
export type ExecutionFactoryCreationOptions = {
resource?: Uri;
pythonPath?: string;
/**
* Whether to execute using `NodeJS.child_process` API, considered `true` as default.
*/
executeAsAProcess?: boolean;
};
export type ExecutionFactoryCreateWithEnvironmentOptions = {
resource?: Uri;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ export class Conda {
return envList.find((e) => isParentPath(executable, e.prefix));
}

public async getRunPythonArgs(env: CondaEnvInfo): Promise<string[] | undefined> {
public async getRunPythonArgs(env: CondaEnvInfo, executeAsAProcess = true): Promise<string[] | undefined> {
const condaVersion = await this.getCondaVersion();
if (condaVersion && lt(condaVersion, CONDA_RUN_VERSION)) {
return undefined;
Expand All @@ -416,7 +416,11 @@ export class Conda {
} else {
args.push('-p', env.prefix);
}
return [this.command, 'run', ...args, '--no-capture-output', '--live-stream', 'python', OUTPUT_MARKER_SCRIPT];
const pythonArgs = [this.command, 'run', ...args, '--no-capture-output', '--live-stream', 'python'];
if (executeAsAProcess) {
pythonArgs.push(OUTPUT_MARKER_SCRIPT);
}
return pythonArgs;
}

/**
Expand Down
11 changes: 10 additions & 1 deletion src/client/terminals/codeExecution/djangoShellCodeExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Disposable, Uri } from 'vscode';
import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
import '../../common/extensions';
import { IFileSystem, IPlatformService } from '../../common/platform/types';
import { IPythonExecutionFactory } from '../../common/process/types';
import { ITerminalServiceFactory } from '../../common/terminal/types';
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
import { copyPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/exec';
Expand All @@ -26,8 +27,16 @@ export class DjangoShellCodeExecutionProvider extends TerminalCodeExecutionProvi
@inject(ICommandManager) commandManager: ICommandManager,
@inject(IFileSystem) fileSystem: IFileSystem,
@inject(IDisposableRegistry) disposableRegistry: Disposable[],
@inject(IPythonExecutionFactory) pythonExecutionFactory: IPythonExecutionFactory,
) {
super(terminalServiceFactory, configurationService, workspace, disposableRegistry, platformService);
super(
terminalServiceFactory,
configurationService,
workspace,
disposableRegistry,
platformService,
pythonExecutionFactory,
);
this.terminalTitle = 'Django Shell';
disposableRegistry.push(new DjangoContextInitializer(documentManager, workspace, fileSystem, commandManager));
}
Expand Down
11 changes: 10 additions & 1 deletion src/client/terminals/codeExecution/repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { inject, injectable } from 'inversify';
import { Disposable } from 'vscode';
import { IWorkspaceService } from '../../common/application/types';
import { IPlatformService } from '../../common/platform/types';
import { IPythonExecutionFactory } from '../../common/process/types';
import { ITerminalServiceFactory } from '../../common/terminal/types';
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
import { TerminalCodeExecutionProvider } from './terminalCodeExecution';
Expand All @@ -19,8 +20,16 @@ export class ReplProvider extends TerminalCodeExecutionProvider {
@inject(IWorkspaceService) workspace: IWorkspaceService,
@inject(IDisposableRegistry) disposableRegistry: Disposable[],
@inject(IPlatformService) platformService: IPlatformService,
@inject(IPythonExecutionFactory) pythonExecutionFactory: IPythonExecutionFactory,
) {
super(terminalServiceFactory, configurationService, workspace, disposableRegistry, platformService);
super(
terminalServiceFactory,
configurationService,
workspace,
disposableRegistry,
platformService,
pythonExecutionFactory,
);
this.terminalTitle = 'REPL';
}
}
11 changes: 7 additions & 4 deletions src/client/terminals/codeExecution/terminalCodeExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Disposable, Uri } from 'vscode';
import { IWorkspaceService } from '../../common/application/types';
import '../../common/extensions';
import { IPlatformService } from '../../common/platform/types';
import { IPythonExecutionFactory } from '../../common/process/types';
import { ITerminalService, ITerminalServiceFactory } from '../../common/terminal/types';
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
import { buildPythonExecInfo, PythonExecInfo } from '../../pythonEnvironments/exec';
Expand All @@ -26,6 +27,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
@inject(IWorkspaceService) protected readonly workspace: IWorkspaceService,
@inject(IDisposableRegistry) protected readonly disposables: Disposable[],
@inject(IPlatformService) protected readonly platformService: IPlatformService,
@inject(IPythonExecutionFactory) private readonly pythonExecutionFactory: IPythonExecutionFactory,
) {}

public async executeFile(file: Uri) {
Expand Down Expand Up @@ -61,11 +63,12 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {

public async getExecutableInfo(resource?: Uri, args: string[] = []): Promise<PythonExecInfo> {
const pythonSettings = this.configurationService.getSettings(resource);
const command = this.platformService.isWindows
? pythonSettings.pythonPath.replace(/\\/g, '/')
: pythonSettings.pythonPath;
const executionFactory = await this.pythonExecutionFactory.create({ resource, executeAsAProcess: false });
const execInfo = executionFactory.getExecutionInfo();
const pythonArgs = execInfo.args;
const command = this.platformService.isWindows ? execInfo.command.replace(/\\/g, '/') : execInfo.command;
const launchArgs = pythonSettings.terminal.launchArgs;
return buildPythonExecInfo(command, [...launchArgs, ...args]);
return buildPythonExecInfo(command, [...pythonArgs, ...launchArgs, ...args]);
}

// Overridden in subclasses, see djangoShellCodeExecution.ts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ suite('Terminal - Django Shell Code Execution', () => {
commandManager.object,
fileSystem.object,
disposables,
pythonExecutionFactory.object,
);

terminalFactory.setup((f) => f.getTerminalService(TypeMoq.It.isAny())).returns(() => terminalService.object);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ suite('Terminal - Code Execution', () => {
terminalFactory = TypeMoq.Mock.ofType<ITerminalServiceFactory>();
terminalSettings = TypeMoq.Mock.ofType<ITerminalSettings>();
terminalService = TypeMoq.Mock.ofType<ITerminalService>();
pythonExecutionFactory = TypeMoq.Mock.ofType<IPythonExecutionFactory>();
const configService = TypeMoq.Mock.ofType<IConfigurationService>();
workspace = TypeMoq.Mock.ofType<IWorkspaceService>();
platform = TypeMoq.Mock.ofType<IPlatformService>();
workspaceFolder = TypeMoq.Mock.ofType<WorkspaceFolder>();
documentManager = TypeMoq.Mock.ofType<IDocumentManager>();
commandManager = TypeMoq.Mock.ofType<ICommandManager>();
fileSystem = TypeMoq.Mock.ofType<IFileSystem>();
pythonExecutionFactory = TypeMoq.Mock.ofType<IPythonExecutionFactory>();
settings = TypeMoq.Mock.ofType<IPythonSettings>();
settings.setup((s) => s.terminal).returns(() => terminalSettings.object);
configService.setup((c) => c.getSettings(TypeMoq.It.isAny())).returns(() => settings.object);
Expand All @@ -79,6 +79,7 @@ suite('Terminal - Code Execution', () => {
workspace.object,
disposables,
platform.object,
pythonExecutionFactory.object,
);
break;
}
Expand All @@ -89,6 +90,7 @@ suite('Terminal - Code Execution', () => {
workspace.object,
disposables,
platform.object,
pythonExecutionFactory.object,
);
expectedTerminalTitle = 'REPL';
break;
Expand All @@ -111,6 +113,7 @@ suite('Terminal - Code Execution', () => {
commandManager.object,
fileSystem.object,
disposables,
pythonExecutionFactory.object,
);
expectedTerminalTitle = 'Django Shell';
break;
Expand Down