Skip to content

Commit 31ce50c

Browse files
author
Kartik Raj
authored
Register or access old Conda locator only when not in experiment (#15292)
* Implemented CondaService using conda.ts * Fix tests * Access CondaLocatorService conditionally in conda activation * Access CondaLocatorService conditionally in conda installer * Fix ESLint errors * Access CondaLocatorService conditionally in python execution factory * Access CondaLocatorService conditionally in TerminalHelper * Only register CondaLocatorService when not in experiment * Code reviews
1 parent e7d9900 commit 31ce50c

File tree

16 files changed

+297
-99
lines changed

16 files changed

+297
-99
lines changed

.eslintignore

-5
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,8 @@ src/test/common/crypto.unit.test.ts
179179
src/test/common/configuration/service.unit.test.ts
180180
src/test/common/net/fileDownloader.unit.test.ts
181181
src/test/common/net/httpClient.unit.test.ts
182-
src/test/common/moduleInstaller.test.ts
183182
src/test/common/terminals/activator/index.unit.test.ts
184183
src/test/common/terminals/activator/base.unit.test.ts
185-
src/test/common/terminals/activation.conda.unit.test.ts
186184
src/test/common/terminals/shellDetector.unit.test.ts
187185
src/test/common/terminals/service.unit.test.ts
188186
src/test/common/terminals/helper.unit.test.ts
@@ -221,7 +219,6 @@ src/test/common/installer/channelManager.unit.test.ts
221219
src/test/common/installer/installer.unit.test.ts
222220
src/test/common/installer/pipInstaller.unit.test.ts
223221
src/test/common/installer/installer.invalidPath.unit.test.ts
224-
src/test/common/installer/moduleInstaller.unit.test.ts
225222
src/test/common/installer/pipEnvInstaller.unit.test.ts
226223
src/test/common/installer/productPath.unit.test.ts
227224
src/test/common/installer/extensionBuildInstaller.unit.test.ts
@@ -558,7 +555,6 @@ src/client/common/terminal/shellDetectors/settingsShellDetector.ts
558555
src/client/common/terminal/shellDetectors/baseShellDetector.ts
559556
src/client/common/terminal/environmentActivationProviders/pipEnvActivationProvider.ts
560557
src/client/common/terminal/environmentActivationProviders/baseActivationProvider.ts
561-
src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts
562558
src/client/common/terminal/environmentActivationProviders/commandPrompt.ts
563559
src/client/common/terminal/environmentActivationProviders/bash.ts
564560
src/client/common/terminal/environmentActivationProviders/pyenvActivationProvider.ts
@@ -620,7 +616,6 @@ src/client/common/application/terminalManager.ts
620616
src/client/common/application/applicationEnvironment.ts
621617
src/client/common/errors/errorUtils.ts
622618
src/client/common/installer/serviceRegistry.ts
623-
src/client/common/installer/condaInstaller.ts
624619
src/client/common/installer/extensionBuildInstaller.ts
625620
src/client/common/installer/productInstaller.ts
626621
src/client/common/installer/channelManager.ts

src/client/common/installer/condaInstaller.ts

+16-5
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1+
/* eslint-disable class-methods-use-this */
12
// Copyright (c) Microsoft Corporation. All rights reserved.
23
// Licensed under the MIT License.
34

45
import { inject, injectable } from 'inversify';
5-
import { ICondaService, ICondaLocatorService } from '../../interpreter/contracts';
6+
import { ICondaService, ICondaLocatorService, IComponentAdapter } from '../../interpreter/contracts';
67
import { IServiceContainer } from '../../ioc/types';
7-
import { ExecutionInfo, IConfigurationService } from '../types';
8+
import { inDiscoveryExperiment } from '../experiments/helpers';
9+
import { ExecutionInfo, IConfigurationService, IExperimentService } from '../types';
810
import { isResource } from '../utils/misc';
911
import { ModuleInstaller } from './moduleInstaller';
1012
import { InterpreterUri } from './types';
@@ -16,6 +18,9 @@ import { InterpreterUri } from './types';
1618
export class CondaInstaller extends ModuleInstaller {
1719
public _isCondaAvailable: boolean | undefined;
1820

21+
// Unfortunately inversify requires the number of args in constructor to be explictly
22+
// specified as more than its base class. So we need the constructor.
23+
// eslint-disable-next-line @typescript-eslint/no-useless-constructor
1924
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
2025
super(serviceContainer);
2126
}
@@ -24,7 +29,7 @@ export class CondaInstaller extends ModuleInstaller {
2429
return 'Conda';
2530
}
2631

27-
public get displayName() {
32+
public get displayName(): string {
2833
return 'Conda';
2934
}
3035

@@ -67,7 +72,10 @@ export class CondaInstaller extends ModuleInstaller {
6772
const pythonPath = isResource(resource)
6873
? this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource).pythonPath
6974
: resource.path;
70-
const condaLocatorService = this.serviceContainer.get<ICondaLocatorService>(ICondaLocatorService);
75+
const experimentService = this.serviceContainer.get<IExperimentService>(IExperimentService);
76+
const condaLocatorService = (await inDiscoveryExperiment(experimentService))
77+
? this.serviceContainer.get<IComponentAdapter>(IComponentAdapter)
78+
: this.serviceContainer.get<ICondaLocatorService>(ICondaLocatorService);
7179
const info = await condaLocatorService.getCondaEnvironment(pythonPath);
7280
const args = [isUpgrade ? 'update' : 'install'];
7381

@@ -97,7 +105,10 @@ export class CondaInstaller extends ModuleInstaller {
97105
* Is the provided interprter a conda environment
98106
*/
99107
private async isCurrentEnvironmentACondaEnvironment(resource?: InterpreterUri): Promise<boolean> {
100-
const condaService = this.serviceContainer.get<ICondaLocatorService>(ICondaLocatorService);
108+
const experimentService = this.serviceContainer.get<IExperimentService>(IExperimentService);
109+
const condaService = (await inDiscoveryExperiment(experimentService))
110+
? this.serviceContainer.get<IComponentAdapter>(IComponentAdapter)
111+
: this.serviceContainer.get<ICondaLocatorService>(ICondaLocatorService);
101112
const pythonPath = isResource(resource)
102113
? this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(resource).pythonPath
103114
: resource.path;

src/client/common/process/pythonExecutionFactory.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,12 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
112112
const processServicePromise = processService
113113
? Promise.resolve(processService)
114114
: this.processServiceFactory.create(resource);
115+
const condaLocatorService = (await inDiscoveryExperiment(this.experimentService))
116+
? this.serviceContainer.get<IComponentAdapter>(IComponentAdapter)
117+
: this.serviceContainer.get<ICondaLocatorService>(ICondaLocatorService);
115118
const [condaVersion, condaEnvironment, condaFile, procService] = await Promise.all([
116119
this.condaService.getCondaVersion(),
117-
this.serviceContainer.get<ICondaLocatorService>(ICondaLocatorService).getCondaEnvironment(pythonPath),
120+
condaLocatorService.getCondaEnvironment(pythonPath),
118121
this.condaService.getCondaFile(),
119122
processServicePromise,
120123
]);

src/client/common/terminal/environmentActivationProviders/condaActivationProvider.ts

+38-34
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
3+
34
'use strict';
5+
46
import '../../extensions';
57

68
import { inject, injectable } from 'inversify';
79
import * as path from 'path';
810
import { Uri } from 'vscode';
911

10-
import { ICondaLocatorService, ICondaService } from '../../../interpreter/contracts';
12+
import { IComponentAdapter, ICondaLocatorService, ICondaService } from '../../../interpreter/contracts';
1113
import { IPlatformService } from '../../platform/types';
12-
import { IConfigurationService } from '../../types';
14+
import { IConfigurationService, IExperimentService } from '../../types';
1315
import { ITerminalActivationCommandProvider, TerminalShellType } from '../types';
1416
import { IServiceContainer } from '../../../ioc/types';
17+
import { inDiscoveryExperiment } from '../../experiments/helpers';
1518

1619
// Version number of conda that requires we call activate with 'conda activate' instead of just 'activate'
1720
const CondaRequiredMajor = 4;
@@ -28,12 +31,15 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
2831
@inject(IPlatformService) private platform: IPlatformService,
2932
@inject(IConfigurationService) private configService: IConfigurationService,
3033
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
34+
@inject(IExperimentService) private experimentService: IExperimentService,
35+
@inject(IComponentAdapter) private pyenvs: IComponentAdapter,
3136
) {}
3237

3338
/**
3439
* Is the given shell supported for activating a conda env?
3540
*/
36-
public isShellSupported(_targetShell: TerminalShellType): boolean {
41+
// eslint-disable-next-line class-methods-use-this
42+
public isShellSupported(): boolean {
3743
return true;
3844
}
3945

@@ -44,7 +50,7 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
4450
resource: Uri | undefined,
4551
targetShell: TerminalShellType,
4652
): Promise<string[] | undefined> {
47-
const pythonPath = this.configService.getSettings(resource).pythonPath;
53+
const { pythonPath } = this.configService.getSettings(resource);
4854
return this.getActivationCommandsForInterpreter(pythonPath, targetShell);
4955
}
5056

@@ -56,10 +62,12 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
5662
pythonPath: string,
5763
targetShell: TerminalShellType,
5864
): Promise<string[] | undefined> {
59-
const condaLocatorService = this.serviceContainer.get<ICondaLocatorService>(ICondaLocatorService);
65+
const condaLocatorService = (await inDiscoveryExperiment(this.experimentService))
66+
? this.pyenvs
67+
: this.serviceContainer.get<ICondaLocatorService>(ICondaLocatorService);
6068
const envInfo = await condaLocatorService.getCondaEnvironment(pythonPath);
6169
if (!envInfo) {
62-
return;
70+
return undefined;
6371
}
6472

6573
const condaEnv = envInfo.name.length > 0 ? envInfo.name : envInfo.path;
@@ -75,7 +83,7 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
7583
versionInfo.minor >= CondaRequiredMinorForPowerShell &&
7684
(targetShell === TerminalShellType.powershell || targetShell === TerminalShellType.powershellCore)
7785
) {
78-
return this.getPowershellCommands(condaEnv);
86+
return _getPowershellCommands(condaEnv);
7987
}
8088
if (versionInfo.minor >= CondaRequiredMinor) {
8189
// New version.
@@ -91,23 +99,22 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
9199
switch (targetShell) {
92100
case TerminalShellType.powershell:
93101
case TerminalShellType.powershellCore:
94-
return this.getPowershellCommands(condaEnv);
102+
return _getPowershellCommands(condaEnv);
95103

96104
// TODO: Do we really special-case fish on Windows?
97105
case TerminalShellType.fish:
98-
return this.getFishCommands(condaEnv, await this.condaService.getCondaFile());
106+
return getFishCommands(condaEnv, await this.condaService.getCondaFile());
99107

100108
default:
101109
if (this.platform.isWindows) {
102110
return this.getWindowsCommands(condaEnv);
103-
} else {
104-
return this.getUnixCommands(condaEnv, await this.condaService.getCondaFile());
105111
}
112+
return getUnixCommands(condaEnv, await this.condaService.getCondaFile());
106113
}
107114
}
108115

109116
public async getWindowsActivateCommand(): Promise<string> {
110-
let activateCmd: string = 'activate';
117+
let activateCmd = 'activate';
111118

112119
const condaExePath = await this.condaService.getCondaFile();
113120

@@ -125,28 +132,25 @@ export class CondaActivationCommandProvider implements ITerminalActivationComman
125132
const activate = await this.getWindowsActivateCommand();
126133
return [`${activate} ${condaEnv.toCommandArgument()}`];
127134
}
128-
/**
129-
* The expectation is for the user to configure Powershell for Conda.
130-
* Hence we just send the command `conda activate ...`.
131-
* This configuration is documented on Conda.
132-
* Extension will not attempt to work around issues by trying to setup shell for user.
133-
*
134-
* @param {string} condaEnv
135-
* @returns {(Promise<string[] | undefined>)}
136-
* @memberof CondaActivationCommandProvider
137-
*/
138-
public async getPowershellCommands(condaEnv: string): Promise<string[] | undefined> {
139-
return [`conda activate ${condaEnv.toCommandArgument()}`];
140-
}
135+
}
141136

142-
public async getFishCommands(condaEnv: string, condaFile: string): Promise<string[] | undefined> {
143-
// https://github.com/conda/conda/blob/be8c08c083f4d5e05b06bd2689d2cd0d410c2ffe/shell/etc/fish/conf.d/conda.fish#L18-L28
144-
return [`${condaFile.fileToCommandArgument()} activate ${condaEnv.toCommandArgument()}`];
145-
}
137+
/**
138+
* The expectation is for the user to configure Powershell for Conda.
139+
* Hence we just send the command `conda activate ...`.
140+
* This configuration is documented on Conda.
141+
* Extension will not attempt to work around issues by trying to setup shell for user.
142+
*/
143+
export async function _getPowershellCommands(condaEnv: string): Promise<string[] | undefined> {
144+
return [`conda activate ${condaEnv.toCommandArgument()}`];
145+
}
146146

147-
public async getUnixCommands(condaEnv: string, condaFile: string): Promise<string[] | undefined> {
148-
const condaDir = path.dirname(condaFile);
149-
const activateFile = path.join(condaDir, 'activate');
150-
return [`source ${activateFile.fileToCommandArgument()} ${condaEnv.toCommandArgument()}`];
151-
}
147+
async function getFishCommands(condaEnv: string, condaFile: string): Promise<string[] | undefined> {
148+
// https://github.com/conda/conda/blob/be8c08c083f4d5e05b06bd2689d2cd0d410c2ffe/shell/etc/fish/conf.d/conda.fish#L18-L28
149+
return [`${condaFile.fileToCommandArgument()} activate ${condaEnv.toCommandArgument()}`];
150+
}
151+
152+
async function getUnixCommands(condaEnv: string, condaFile: string): Promise<string[] | undefined> {
153+
const condaDir = path.dirname(condaFile);
154+
const activateFile = path.join(condaDir, 'activate');
155+
return [`source ${activateFile.fileToCommandArgument()} ${condaEnv.toCommandArgument()}`];
152156
}

src/client/common/terminal/helper.ts

+10-4
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,17 @@
33

44
import { inject, injectable, multiInject, named } from 'inversify';
55
import { Terminal, Uri } from 'vscode';
6-
import { ICondaLocatorService, IInterpreterService } from '../../interpreter/contracts';
6+
import { IComponentAdapter, ICondaLocatorService, IInterpreterService } from '../../interpreter/contracts';
7+
import { IServiceContainer } from '../../ioc/types';
78
import { EnvironmentType, PythonEnvironment } from '../../pythonEnvironments/info';
89
import { sendTelemetryEvent } from '../../telemetry';
910
import { EventName } from '../../telemetry/constants';
1011
import { ITerminalManager } from '../application/types';
12+
import { inDiscoveryExperiment } from '../experiments/helpers';
1113
import '../extensions';
1214
import { traceDecorators, traceError } from '../logger';
1315
import { IPlatformService } from '../platform/types';
14-
import { IConfigurationService, Resource } from '../types';
16+
import { IConfigurationService, IExperimentService, Resource } from '../types';
1517
import { OSType } from '../utils/platform';
1618
import { ShellDetector } from './shellDetector';
1719
import {
@@ -28,7 +30,7 @@ export class TerminalHelper implements ITerminalHelper {
2830
constructor(
2931
@inject(IPlatformService) private readonly platform: IPlatformService,
3032
@inject(ITerminalManager) private readonly terminalManager: ITerminalManager,
31-
@inject(ICondaLocatorService) private readonly condaService: ICondaLocatorService,
33+
@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer,
3234
@inject(IInterpreterService) readonly interpreterService: IInterpreterService,
3335
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
3436
@inject(ITerminalActivationCommandProvider)
@@ -129,10 +131,14 @@ export class TerminalHelper implements ITerminalHelper {
129131
): Promise<string[] | undefined> {
130132
const settings = this.configurationService.getSettings(resource);
131133

134+
const experimentService = this.serviceContainer.get<IExperimentService>(IExperimentService);
135+
const condaService = (await inDiscoveryExperiment(experimentService))
136+
? this.serviceContainer.get<IComponentAdapter>(IComponentAdapter)
137+
: this.serviceContainer.get<ICondaLocatorService>(ICondaLocatorService);
132138
// If we have a conda environment, then use that.
133139
const isCondaEnvironment = interpreter
134140
? interpreter.envType === EnvironmentType.Conda
135-
: await this.condaService.isCondaEnvironment(settings.pythonPath);
141+
: await condaService.isCondaEnvironment(settings.pythonPath);
136142
if (isCondaEnvironment) {
137143
const activationCommands = interpreter
138144
? await this.conda.getActivationCommandsForInterpreter(interpreter.path, terminalShellType)

0 commit comments

Comments
 (0)