Add support for "conda run" in PythonExecutionFactory (#8259)

* Initial commit for conda run
* Forgot djangoShellCodeExecution.ts
* Fix news file
* Fix single workspace tests
* Don't use interpreter in terminal code exec
* Try increasing activation timeout time for smoke tests
* Try different timeouts
* try commenting conda in pythonExecutionFactory
* Undo timeout changes
* forgot await lol
* Uncomment things
* Small fixes + remove empty line
* Reduce PR noise by removing some autoformatting
* Update createActivatedEnv + minor refactoring
* Add pythonExecutionFactory unit tests
* Add pythonExecutionFactory unit tests
* Fix conda env name check + django terminal
* Unit tests for terminalCodeExecution.ts
* Fix linting functional tests
* djangoShellCodeExecution tests + fix terminalCodeExec.unit.test.ts
* Make conda path check more explicit
* Add Windows test to PythonExecutionFactory
* Add conda call verification
* Add CondaEnvironmentInfo type
* CondaExecutionService unit tests
* Add comment
* Remove autformatting
* Make getExecutableInfo public
* Use CondaExecutionService in terminal code execution
* Apply suggestions from code review
Co-Authored-By: Eric Snow <[email protected]>
* refactor CondaExecutionService instantiation
* getExecutableInfo unit tests + more concise conda getExecutableInfo
* createCondaExecutionService unit tests
* Add conda version check
* Housekeeping
* Move terminal changes to another PR
* Typo
* Revert execObservable and execModuleObservable
* Update GH link
* More housekeeping
* Fix compilation errors
* Undo formatting changes
* More formatting reverting
* Try creating a conda environment first
* Fix tests
* Drop unnecessary command arg in getExecutionInfo
* Fix PythonDaemon getExecutionInfo
* Undo formatting changes
* Fix merge gone sideways
* Fix linting functional tests
* Fix unit tests
* Revert "Move terminal changes to another PR"
This reverts commit 689bf1f.
* Fixes
* Fix tests
* Fix single workspace tests
* Remove unused declaration
* Move createConda around in createActivatedEnv
* Fix unit tests

Author: kimadeline

news/3 Code Health/7696.md

@@ -0,0 +1,2 @@
+Use "conda run" (instead of using the "python.pythonPath" setting directly) when executing
+Python and an Anaconda environment is selected.

src/client/common/process/condaExecutionService.ts

@@ -0,0 +1,26 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+import { injectable } from 'inversify';
+import { CondaEnvironmentInfo } from '../../interpreter/contracts';
+import { IServiceContainer } from '../../ioc/types';
+import { PythonExecutionService } from './pythonProcess';
+import { IProcessService, PythonExecutionInfo } from './types';
+
+@injectable()
+export class CondaExecutionService extends PythonExecutionService {
+    constructor(
+        serviceContainer: IServiceContainer,
+        procService: IProcessService,
+        pythonPath: string,
+        private readonly condaFile: string,
+        private readonly condaEnvironment: CondaEnvironmentInfo
+    ) {
+        super(serviceContainer, procService, pythonPath);
+    }
+
+    public getExecutionInfo(args: string[]): PythonExecutionInfo {
+        const executionArgs = this.condaEnvironment.name !== '' ? ['-n', this.condaEnvironment.name] : ['-p', this.condaEnvironment.path];
+
+        return { command: this.condaFile, args: ['run', ...executionArgs, 'python', ...args] };
+    }
+}

src/client/common/process/pythonDaemon.ts

@@ -21,6 +21,7 @@ import {
     IPythonExecutionService,
     ObservableExecutionResult,
     Output,
+    PythonExecutionInfo,
     PythonVersionInfo,
     SpawnOptions,
     StdErrError
@@ -96,6 +97,9 @@ export class PythonDaemonExecutionService implements IPythonDaemonExecutionServi
             return this.pythonExecutionService.getExecutablePath();
         }
     }
+    public getExecutionInfo(args: string[]): PythonExecutionInfo {
+        return this.pythonExecutionService.getExecutionInfo(args);
+    }
     public async isModuleInstalled(moduleName: string): Promise<boolean> {
         this.throwIfRPCConnectionIsDead();
         try {

src/client/common/process/pythonDaemonPool.ts

@@ -22,6 +22,7 @@ import {
     IPythonDaemonExecutionService,
     IPythonExecutionService,
     ObservableExecutionResult,
+    PythonExecutionInfo,
     SpawnOptions
 } from './types';
 
@@ -71,6 +72,9 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
         this.logger.logProcess(`${this.pythonPath} (daemon)`, ['getExecutablePath']);
         return this.wrapCall(daemon => daemon.getExecutablePath());
     }
+    public getExecutionInfo(args: string[]): PythonExecutionInfo {
+        return this.pythonExecutionService.getExecutionInfo(args);
+    }
     public async isModuleInstalled(moduleName: string): Promise<boolean> {
         this.logger.logProcess(`${this.pythonPath} (daemon)`, ['-m', moduleName]);
         return this.wrapCall(daemon => daemon.isModuleInstalled(moduleName));

src/client/common/process/pythonExecutionFactory.ts

@@ -1,16 +1,19 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 import { inject, injectable } from 'inversify';
+import { gte } from 'semver';
 
+import { Uri } from 'vscode';
 import { IEnvironmentActivationService } from '../../interpreter/activation/types';
-import { IInterpreterService } from '../../interpreter/contracts';
+import { ICondaService, IInterpreterService } from '../../interpreter/contracts';
 import { WindowsStoreInterpreter } from '../../interpreter/locators/services/windowsStoreInterpreter';
 import { IWindowsStoreInterpreter } from '../../interpreter/locators/types';
 import { IServiceContainer } from '../../ioc/types';
 import { sendTelemetryEvent } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
 import { traceError } from '../logger';
 import { IConfigurationService, IDisposableRegistry } from '../types';
+import { CondaExecutionService } from './condaExecutionService';
 import { ProcessService } from './proc';
 import { PythonDaemonExecutionServicePool } from './pythonDaemonPool';
 import { PythonExecutionService } from './pythonProcess';
@@ -28,6 +31,9 @@ import {
 } from './types';
 import { WindowsStorePythonProcess } from './windowsStorePythonProcess';
 
+// Minimum version number of conda required to be able to use 'conda run'
+export const CONDA_RUN_VERSION = '4.6.0';
+
 @injectable()
 export class PythonExecutionFactory implements IPythonExecutionFactory {
     private readonly daemonsPerPythonService = new Map<string, Promise<IPythonDaemonExecutionService>>();
@@ -36,6 +42,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         @inject(IEnvironmentActivationService) private readonly activationHelper: IEnvironmentActivationService,
         @inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
         @inject(IConfigurationService) private readonly configService: IConfigurationService,
+        @inject(ICondaService) private readonly condaService: ICondaService,
         @inject(IBufferDecoder) private readonly decoder: IBufferDecoder,
         @inject(WindowsStoreInterpreter) private readonly windowsStoreInterpreter: IWindowsStoreInterpreter
     ) {}
@@ -44,6 +51,18 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         const processService: IProcessService = await this.processServiceFactory.create(options.resource);
         const processLogger = this.serviceContainer.get<IProcessLogger>(IProcessLogger);
         processService.on('exec', processLogger.logProcess.bind(processLogger));
+
+        // Don't bother getting a conda execution service instance if we haven't fetched the list of interpreters yet.
+        // Also, without this hasInterpreters check smoke tests will time out
+        const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
+        const hasInterpreters = await interpreterService.hasInterpreters;
+        if (hasInterpreters) {
+            const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService);
+            if (condaExecutionService) {
+                return condaExecutionService;
+            }
+        }
+
         if (this.windowsStoreInterpreter.isWindowsStoreInterpreter(pythonPath)) {
             return new WindowsStorePythonProcess(this.serviceContainer, processService, pythonPath, this.windowsStoreInterpreter);
         }
@@ -98,6 +117,33 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
         const processLogger = this.serviceContainer.get<IProcessLogger>(IProcessLogger);
         processService.on('exec', processLogger.logProcess.bind(processLogger));
         this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry).push(processService);
+
+        const condaExecutionService = await this.createCondaExecutionService(pythonPath, processService);
+        if (condaExecutionService) {
+            return condaExecutionService;
+        }
+
         return new PythonExecutionService(this.serviceContainer, processService, pythonPath);
     }
+    public async createCondaExecutionService(pythonPath: string, processService?: IProcessService, resource?: Uri): Promise<CondaExecutionService | undefined> {
+        const processServicePromise = processService ? Promise.resolve(processService) : this.processServiceFactory.create(resource);
+        const [condaVersion, condaEnvironment, condaFile, procService] = await Promise.all([
+            this.condaService.getCondaVersion(),
+            this.condaService.getCondaEnvironment(pythonPath),
+            this.condaService.getCondaFile(),
+            processServicePromise
+        ]);
+
+        if (condaVersion && gte(condaVersion, CONDA_RUN_VERSION) && condaEnvironment && condaFile && procService) {
+            // Add logging to the newly created process service
+            if (!processService) {
+                const processLogger = this.serviceContainer.get<IProcessLogger>(IProcessLogger);
+                procService.on('exec', processLogger.logProcess.bind(processLogger));
+                this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry).push(procService);
+            }
+            return new CondaExecutionService(this.serviceContainer, procService, pythonPath, condaFile, condaEnvironment);
+        }
+
+        return Promise.resolve(undefined);
+    }
 }

src/client/common/process/pythonProcess.ts

@@ -18,6 +18,7 @@ import {
     IProcessService,
     IPythonExecutionService,
     ObservableExecutionResult,
+    PythonExecutionInfo,
     PythonVersionInfo,
     SpawnOptions
 } from './types';
@@ -41,7 +42,8 @@ export class PythonExecutionService implements IPythonExecutionService {
             // See these two bugs:
             // https://github.com/microsoft/vscode-python/issues/7569
             // https://github.com/microsoft/vscode-python/issues/7760
-            const jsonValue = await waitForPromise(this.procService.exec(this.pythonPath, [file], { mergeStdOutErr: true }), 5000)
+            const { command, args } = this.getExecutionInfo([file]);
+            const jsonValue = await waitForPromise(this.procService.exec(command, args, { mergeStdOutErr: true }), 5000)
                 .then(output => output ? output.stdout.trim() : '--timed out--'); // --timed out-- should cause an exception
 
             let json: { versionInfo: PythonVersionInfo; sysPrefix: string; sysVersion: string; is64Bit: boolean };
@@ -69,29 +71,41 @@ export class PythonExecutionService implements IPythonExecutionService {
         if (await this.fileSystem.fileExists(this.pythonPath)) {
             return this.pythonPath;
         }
-        return this.procService.exec(this.pythonPath, ['-c', 'import sys;print(sys.executable)'], { throwOnStdErr: true })
-            .then(output => output.stdout.trim());
+
+        const { command, args } = this.getExecutionInfo(['-c', 'import sys;print(sys.executable)']);
+        return this.procService.exec(command, args, { throwOnStdErr: true }).then(output => output.stdout.trim());
     }
     public async isModuleInstalled(moduleName: string): Promise<boolean> {
-        return this.procService.exec(this.pythonPath, ['-c', `import ${moduleName}`], { throwOnStdErr: true })
+        const { command, args } = this.getExecutionInfo(['-c', `import ${moduleName}`]);
+        return this.procService.exec(command, args, { throwOnStdErr: true })
             .then(() => true).catch(() => false);
     }
 
+    public getExecutionInfo(args: string[]): PythonExecutionInfo {
+        return { command: this.pythonPath, args };
+    }
+
     public execObservable(args: string[], options: SpawnOptions): ObservableExecutionResult<string> {
         const opts: SpawnOptions = { ...options };
+        // Cannot use this.getExecutionInfo() until 'conda run' can be run without buffering output.
+        // See https://github.com/microsoft/vscode-python/issues/8473
         return this.procService.execObservable(this.pythonPath, args, opts);
     }
     public execModuleObservable(moduleName: string, args: string[], options: SpawnOptions): ObservableExecutionResult<string> {
         const opts: SpawnOptions = { ...options };
+        // Cannot use this.getExecutionInfo() until 'conda run' can be run without buffering output.
+        // See https://github.com/microsoft/vscode-python/issues/8473
         return this.procService.execObservable(this.pythonPath, ['-m', moduleName, ...args], opts);
     }
     public async exec(args: string[], options: SpawnOptions): Promise<ExecutionResult<string>> {
         const opts: SpawnOptions = { ...options };
-        return this.procService.exec(this.pythonPath, args, opts);
+        const executable = this.getExecutionInfo(args);
+        return this.procService.exec(executable.command, executable.args, opts);
     }
     public async execModule(moduleName: string, args: string[], options: SpawnOptions): Promise<ExecutionResult<string>> {
         const opts: SpawnOptions = { ...options };
-        const result = await this.procService.exec(this.pythonPath, ['-m', moduleName, ...args], opts);
+        const executable = this.getExecutionInfo(['-m', moduleName, ...args]);
+        const result = await this.procService.exec(executable.command, executable.args, opts);
 
         // If a module is not installed we'll have something in stderr.
         if (moduleName && ErrorUtils.outputHasModuleNotInstalledError(moduleName!, result.stderr)) {

src/client/common/process/types.ts

@@ -9,6 +9,7 @@ import { Newable } from '../../ioc/types';
 import { ExecutionInfo, IDisposable, Version } from '../types';
 import { Architecture } from '../utils/platform';
 import { EnvironmentVariables } from '../variables/types';
+import { CondaExecutionService } from './condaExecutionService';
 
 export const IBufferDecoder = Symbol('IBufferDecoder');
 export interface IBufferDecoder {
@@ -115,6 +116,7 @@ export interface IPythonExecutionFactory {
      */
     createDaemon(options:  DaemonExecutionFactoryCreationOptions): Promise<IPythonExecutionService>;
     createActivatedEnvironment(options: ExecutionFactoryCreateWithEnvironmentOptions): Promise<IPythonExecutionService>;
+    createCondaExecutionService(pythonPath: string, processService?: IProcessService, resource?: Uri): Promise<CondaExecutionService | undefined>;
 }
 export type ReleaseLevel = 'alpha' | 'beta' | 'candidate' | 'final' | 'unknown';
 export type PythonVersionInfo = [number, number, number, ReleaseLevel];
@@ -132,6 +134,7 @@ export interface IPythonExecutionService {
     getInterpreterInformation(): Promise<InterpreterInfomation | undefined>;
     getExecutablePath(): Promise<string>;
     isModuleInstalled(moduleName: string): Promise<boolean>;
+    getExecutionInfo(args: string[]): PythonExecutionInfo;
 
     execObservable(args: string[], options: SpawnOptions): ObservableExecutionResult<string>;
     execModuleObservable(moduleName: string, args: string[], options: SpawnOptions): ObservableExecutionResult<string>;
@@ -140,6 +143,10 @@ export interface IPythonExecutionService {
     execModule(moduleName: string, args: string[], options: SpawnOptions): Promise<ExecutionResult<string>>;
 }
 
+export type PythonExecutionInfo = {
+    command: string;
+    args: string[];
+};
 /**
  * Identical to the PythonExecutionService, but with a `dispose` method.
  * This is a daemon process that lives on until it is disposed, hence the `IDisposable`.

src/client/interpreter/contracts.ts

@@ -43,6 +43,11 @@ export type CondaInfo = {
     conda_version?: string;
 };
 
+export type CondaEnvironmentInfo = {
+    name: string;
+    path: string;
+};
+
 export const ICondaService = Symbol('ICondaService');
 
 export interface ICondaService {
@@ -51,11 +56,11 @@ export interface ICondaService {
     isCondaAvailable(): Promise<boolean>;
     getCondaVersion(): Promise<SemVer | undefined>;
     getCondaInfo(): Promise<CondaInfo | undefined>;
-    getCondaEnvironments(ignoreCache: boolean): Promise<({ name: string; path: string }[]) | undefined>;
+    getCondaEnvironments(ignoreCache: boolean): Promise<CondaEnvironmentInfo[] | undefined>;
     getInterpreterPath(condaEnvironmentPath: string): string;
     getCondaFileFromInterpreter(interpreterPath?: string, envName?: string): Promise<string | undefined>;
     isCondaEnvironment(interpreterPath: string): Promise<boolean>;
-    getCondaEnvironment(interpreterPath: string): Promise<{ name: string; path: string } | undefined>;
+    getCondaEnvironment(interpreterPath: string): Promise<CondaEnvironmentInfo | undefined>;
 }
 
 export enum InterpreterType {

src/client/interpreter/locators/index.ts

@@ -51,7 +51,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
         return new EventEmitter<Promise<PythonInterpreter[]>>().event;
     }
     public get hasInterpreters(): Promise<boolean> {
-        return this._hasInterpreters.promise;
+        return this._hasInterpreters.completed ? this._hasInterpreters.promise : Promise.resolve(false);
     }
 
     /**

src/client/interpreter/locators/services/cacheableLocatorService.ts

@@ -34,7 +34,7 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ
         return this.locating.event;
     }
     public get hasInterpreters(): Promise<boolean> {
-        return this._hasInterpreters.promise;
+        return this._hasInterpreters.completed ? this._hasInterpreters.promise : Promise.resolve(false);
     }
     public abstract dispose(): void;
     @traceDecorators.verbose('Get Interpreters in CacheableLocatorService')

src/client/interpreter/locators/services/condaService.ts

@@ -10,6 +10,7 @@ import { IProcessServiceFactory } from '../../../common/process/types';
 import { IConfigurationService, IDisposableRegistry, ILogger, IPersistentStateFactory } from '../../../common/types';
 import { cache } from '../../../common/utils/decorators';
 import {
+    CondaEnvironmentInfo,
     CondaInfo,
     ICondaService,
     IInterpreterLocatorService,
@@ -219,10 +220,10 @@ export class CondaService implements ICondaService {
      * Return the list of conda envs (by name, interpreter filename).
      */
     @traceDecorators.verbose('Get Conda environments')
-    public async getCondaEnvironments(ignoreCache: boolean): Promise<({ name: string; path: string }[]) | undefined> {
+    public async getCondaEnvironments(ignoreCache: boolean): Promise<CondaEnvironmentInfo[] | undefined> {
         // Global cache.
         // tslint:disable-next-line:no-any
-        const globalPersistence = this.persistentStateFactory.createGlobalPersistentState<{ data: { name: string; path: string }[] | undefined }>('CONDA_ENVIRONMENTS', undefined as any);
+        const globalPersistence = this.persistentStateFactory.createGlobalPersistentState<{ data: CondaEnvironmentInfo[] | undefined }>('CONDA_ENVIRONMENTS', undefined as any);
         if (!ignoreCache && globalPersistence.value) {
             return globalPersistence.value.data;
         }