Skip to content

Commit 381a6e0

Browse files
author
Kartik Raj
authored
Remove python.pythonPath experiment (#18213)
* Remove DeprecatePythonPath experiment * Fix unit tests * Fix tests for configsettings * Fix mac interpreter tests * Fix activation manager tests * Fix config service tests * Fix startup telemetry tests * Remove all traces of experiment * Fix python path updater tests * Update setting description for `python.defaultInterpreterPath` * Fix single workspace tests attempt#1 * Do not import from client folder in smoke tests * new enttry
1 parent 10bb8d0 commit 381a6e0

File tree

49 files changed

+421
-1395
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+421
-1395
lines changed

‎data/.vscode/settings.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
{
2-
"python.pythonPath": "/usr/bin/python3"
2+
"python.defaultInterpreterPath": "/usr/bin/python3"
33
}

‎news/3 Code Health/17977.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Remove `python.pythonPath` setting and `pythonDeprecatePythonPath` experiment.

‎package.json‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@
455455
},
456456
"python.defaultInterpreterPath": {
457457
"default": "python",
458-
"description": "Path to Python, you can use a custom version of Python by modifying this setting to include the full path. This default setting is used as a fallback if no interpreter is selected for the workspace. The extension will also not set nor change the value of this setting, it will only read from it.",
458+
"description": "Path to default Python to use when extension loads up for the first time, no longer used once an interpreter is selected for the workspace. See https://aka.ms/AAfekmf to understand when this is used.",
459459
"scope": "machine-overridable",
460460
"type": "string"
461461
},
@@ -982,12 +982,6 @@
982982
"scope": "machine-overridable",
983983
"type": "string"
984984
},
985-
"python.pythonPath": {
986-
"default": "python",
987-
"description": "(DEPRECATED: Note this setting is not used when in pythonDeprecatePythonPath experiment) Path to Python, you can use a custom version of Python by modifying this setting to include the full path.",
988-
"scope": "machine-overridable",
989-
"type": "string"
990-
},
991985
"python.sortImports.args": {
992986
"default": [],
993987
"description": "Arguments passed in. Each argument is a separate item in the array.",

‎src/client/activation/activationManager.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,8 @@ import { TextDocument } from 'vscode';
88
import { IApplicationDiagnostics } from '../application/types';
99
import { IActiveResourceService, IDocumentManager, IWorkspaceService } from '../common/application/types';
1010
import { PYTHON_LANGUAGE } from '../common/constants';
11-
import { DeprecatePythonPath } from '../common/experiments/groups';
1211
import { IFileSystem } from '../common/platform/types';
13-
import { IDisposable, IExperimentService, IInterpreterPathService, Resource } from '../common/types';
12+
import { IDisposable, Resource } from '../common/types';
1413
import { Deferred } from '../common/utils/async';
1514
import { IInterpreterAutoSelectionService } from '../interpreter/autoSelection/types';
1615
import { traceDecoratorError } from '../logging';
@@ -37,8 +36,6 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
3736
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
3837
@inject(IFileSystem) private readonly fileSystem: IFileSystem,
3938
@inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService,
40-
@inject(IExperimentService) private readonly experiments: IExperimentService,
41-
@inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService,
4239
) {
4340
if (!this.workspaceService.isTrusted) {
4441
this.activationServices = this.activationServices.filter(
@@ -90,9 +87,6 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
9087

9188
if (this.workspaceService.isTrusted) {
9289
// Do not interact with interpreters in a untrusted workspace.
93-
if (this.experiments.inExperimentSync(DeprecatePythonPath.experiment)) {
94-
await this.interpreterPathService.copyOldInterpreterStorageValuesToNew(resource);
95-
}
9690
await this.autoSelection.autoSelectInterpreter(resource);
9791
}
9892
await sendActivationTelemetry(this.fileSystem, this.workspaceService, resource);

‎src/client/activation/jedi/languageServerProxy.ts‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ import {
1111
} from 'vscode-languageclient/node';
1212

1313
import { ChildProcess } from 'child_process';
14-
import { DeprecatePythonPath } from '../../common/experiments/groups';
15-
import { IExperimentService, IInterpreterPathService, Resource } from '../../common/types';
14+
import { IInterpreterPathService, Resource } from '../../common/types';
1615
import { PythonEnvironment } from '../../pythonEnvironments/info';
1716
import { captureTelemetry } from '../../telemetry';
1817
import { EventName } from '../../telemetry/constants';
@@ -37,7 +36,6 @@ export class JediLanguageServerProxy implements ILanguageServerProxy {
3736

3837
constructor(
3938
@inject(ILanguageClientFactory) private readonly factory: ILanguageClientFactory,
40-
@inject(IExperimentService) private readonly experiments: IExperimentService,
4139
@inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService,
4240
) {}
4341

@@ -148,18 +146,16 @@ export class JediLanguageServerProxy implements ILanguageServerProxy {
148146
const progressReporting = new ProgressReporting(this.languageClient!);
149147
this.disposables.push(progressReporting);
150148

151-
if (this.experiments.inExperimentSync(DeprecatePythonPath.experiment)) {
152-
this.disposables.push(
153-
this.interpreterPathService.onDidChange(() => {
154-
// Manually send didChangeConfiguration in order to get the server to re-query
155-
// the workspace configurations (to then pick up pythonPath set in the middleware).
156-
// This is needed as interpreter changes via the interpreter path service happen
157-
// outside of VS Code's settings (which would mean VS Code sends the config updates itself).
158-
this.languageClient!.sendNotification(DidChangeConfigurationNotification.type, {
159-
settings: null,
160-
});
161-
}),
162-
);
163-
}
149+
this.disposables.push(
150+
this.interpreterPathService.onDidChange(() => {
151+
// Manually send didChangeConfiguration in order to get the server to re-query
152+
// the workspace configurations (to then pick up pythonPath set in the middleware).
153+
// This is needed as interpreter changes via the interpreter path service happen
154+
// outside of VS Code's settings (which would mean VS Code sends the config updates itself).
155+
this.languageClient!.sendNotification(DidChangeConfigurationNotification.type, {
156+
settings: null,
157+
});
158+
}),
159+
);
164160
}
165161
}

‎src/client/activation/node/languageServerProxy.ts‎

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
State,
1212
} from 'vscode-languageclient/node';
1313

14-
import { DeprecatePythonPath } from '../../common/experiments/groups';
1514
import { IConfigurationService, IExperimentService, IInterpreterPathService, Resource } from '../../common/types';
1615
import { createDeferred, Deferred, sleep } from '../../common/utils/async';
1716
import { noop } from '../../common/utils/misc';
@@ -177,20 +176,17 @@ export class NodeLanguageServerProxy implements ILanguageServerProxy {
177176
const progressReporting = new ProgressReporting(this.languageClient!);
178177
this.disposables.push(progressReporting);
179178

180-
if (this.experimentService.inExperimentSync(DeprecatePythonPath.experiment)) {
181-
this.disposables.push(
182-
this.interpreterPathService.onDidChange(() => {
183-
// Manually send didChangeConfiguration in order to get the server to requery
184-
// the workspace configurations (to then pick up pythonPath set in the middleware).
185-
// This is needed as interpreter changes via the interpreter path service happen
186-
// outside of VS Code's settings (which would mean VS Code sends the config updates itself).
187-
this.languageClient!.sendNotification(DidChangeConfigurationNotification.type, {
188-
settings: null,
189-
});
190-
}),
191-
);
192-
}
193-
179+
this.disposables.push(
180+
this.interpreterPathService.onDidChange(() => {
181+
// Manually send didChangeConfiguration in order to get the server to requery
182+
// the workspace configurations (to then pick up pythonPath set in the middleware).
183+
// This is needed as interpreter changes via the interpreter path service happen
184+
// outside of VS Code's settings (which would mean VS Code sends the config updates itself).
185+
this.languageClient!.sendNotification(DidChangeConfigurationNotification.type, {
186+
settings: null,
187+
});
188+
}),
189+
);
194190
this.disposables.push(
195191
this.environmentService.onDidEnvironmentVariablesChange(() => {
196192
this.languageClient!.sendNotification(DidChangeConfigurationNotification.type, {

‎src/client/application/diagnostics/checks/macPythonInterpreter.ts‎

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,12 @@
33

44
// eslint-disable-next-line max-classes-per-file
55
import { inject, injectable } from 'inversify';
6-
import { ConfigurationChangeEvent, DiagnosticSeverity, Uri } from 'vscode';
7-
import { IWorkspaceService } from '../../../common/application/types';
8-
import { DeprecatePythonPath } from '../../../common/experiments/groups';
6+
import { DiagnosticSeverity } from 'vscode';
97
import '../../../common/extensions';
108
import { IPlatformService } from '../../../common/platform/types';
119
import {
1210
IConfigurationService,
1311
IDisposableRegistry,
14-
IExperimentService,
1512
IInterpreterPathService,
1613
InterpreterConfigurationScope,
1714
Resource,
@@ -148,40 +145,15 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
148145
}
149146

150147
protected addPythonPathChangedHandler(): void {
151-
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
152148
const disposables = this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
153149
const interpreterPathService = this.serviceContainer.get<IInterpreterPathService>(IInterpreterPathService);
154-
const experiments = this.serviceContainer.get<IExperimentService>(IExperimentService);
155-
if (experiments.inExperimentSync(DeprecatePythonPath.experiment)) {
156-
disposables.push(interpreterPathService.onDidChange((i) => this.onDidChangeConfiguration(undefined, i)));
157-
}
158-
disposables.push(workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this)));
150+
disposables.push(interpreterPathService.onDidChange((i) => this.onDidChangeConfiguration(i)));
159151
}
160152

161153
protected async onDidChangeConfiguration(
162-
event?: ConfigurationChangeEvent,
163-
interpreterConfigurationScope?: InterpreterConfigurationScope,
154+
interpreterConfigurationScope: InterpreterConfigurationScope,
164155
): Promise<void> {
165-
let workspaceUri: Resource;
166-
if (event) {
167-
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
168-
const workspacesUris: (Uri | undefined)[] = workspaceService.hasWorkspaceFolders
169-
? workspaceService.workspaceFolders!.map((workspace) => workspace.uri)
170-
: [undefined];
171-
const workspaceUriIndex = workspacesUris.findIndex((uri) =>
172-
event.affectsConfiguration('python.pythonPath', uri),
173-
);
174-
if (workspaceUriIndex === -1) {
175-
return;
176-
}
177-
workspaceUri = workspacesUris[workspaceUriIndex];
178-
} else if (interpreterConfigurationScope) {
179-
workspaceUri = interpreterConfigurationScope.uri;
180-
} else {
181-
throw new Error(
182-
'One of `interpreterConfigurationScope` or `event` should be defined when calling `onDidChangeConfiguration`.',
183-
);
184-
}
156+
const workspaceUri = interpreterConfigurationScope.uri;
185157
// Lets wait, for more changes, dirty simple throttling.
186158
if (this.timeOut && typeof this.timeOut !== 'number') {
187159
clearTimeout(this.timeOut);

‎src/client/application/diagnostics/checks/pythonPathDeprecated.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@
55
import { inject, named } from 'inversify';
66
import { DiagnosticSeverity } from 'vscode';
77
import { IWorkspaceService } from '../../../common/application/types';
8-
import { DeprecatePythonPath } from '../../../common/experiments/groups';
9-
import { IDisposableRegistry, IExperimentService, Resource } from '../../../common/types';
8+
import { IDisposableRegistry, Resource } from '../../../common/types';
109
import { Common, Diagnostics } from '../../../common/utils/localize';
1110
import { IServiceContainer } from '../../../ioc/types';
1211
import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
@@ -44,10 +43,6 @@ export class PythonPathDeprecatedDiagnosticService extends BaseDiagnosticsServic
4443
}
4544

4645
public async diagnose(resource: Resource): Promise<IDiagnostic[]> {
47-
const experiments = this.serviceContainer.get<IExperimentService>(IExperimentService);
48-
if (!experiments.inExperimentSync(DeprecatePythonPath.experiment)) {
49-
return [];
50-
}
5146
const setting = this.workspaceService.getConfiguration('python', resource).inspect<string>('pythonPath');
5247
if (!setting) {
5348
return [];

‎src/client/application/diagnostics/checks/upgradeCodeRunner.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ import { inject, named } from 'inversify';
66
import { DiagnosticSeverity } from 'vscode';
77
import { IWorkspaceService } from '../../../common/application/types';
88
import { CODE_RUNNER_EXTENSION_ID } from '../../../common/constants';
9-
import { DeprecatePythonPath } from '../../../common/experiments/groups';
10-
import { IDisposableRegistry, IExperimentService, IExtensions, Resource } from '../../../common/types';
9+
import { IDisposableRegistry, IExtensions, Resource } from '../../../common/types';
1110
import { Common, Diagnostics } from '../../../common/utils/localize';
1211
import { IServiceContainer } from '../../../ioc/types';
1312
import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
@@ -51,10 +50,6 @@ export class UpgradeCodeRunnerDiagnosticService extends BaseDiagnosticsService {
5150
if (this._diagnosticReturned) {
5251
return [];
5352
}
54-
const experiments = this.serviceContainer.get<IExperimentService>(IExperimentService);
55-
if (!experiments.inExperimentSync(DeprecatePythonPath.experiment)) {
56-
return [];
57-
}
5853
const extension = this.extensions.getExtension(CODE_RUNNER_EXTENSION_ID);
5954
if (!extension) {
6055
return [];

‎src/client/common/configSettings.ts‎

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,12 @@ import { ITestingSettings } from '../testing/configuration/types';
2323
import { IWorkspaceService } from './application/types';
2424
import { WorkspaceService } from './application/workspace';
2525
import { DEFAULT_INTERPRETER_SETTING, isTestExecution } from './constants';
26-
import { DeprecatePythonPath } from './experiments/groups';
2726
import { ExtensionChannels } from './insidersBuild/types';
2827
import { IS_WINDOWS } from './platform/constants';
2928
import {
3029
IAutoCompleteSettings,
3130
IDefaultLanguageServer,
3231
IExperiments,
33-
IExperimentService,
3432
IFormattingSettings,
3533
IInterpreterPathService,
3634
ILintingSettings,
@@ -146,10 +144,9 @@ export class PythonSettings implements IPythonSettings {
146144
constructor(
147145
workspaceFolder: Resource,
148146
private readonly interpreterAutoSelectionService: IInterpreterAutoSelectionProxyService,
149-
workspace?: IWorkspaceService,
150-
private readonly experimentsManager?: IExperimentService,
151-
private readonly interpreterPathService?: IInterpreterPathService,
152-
private readonly defaultLS?: IDefaultLanguageServer,
147+
workspace: IWorkspaceService,
148+
private readonly interpreterPathService: IInterpreterPathService,
149+
private readonly defaultLS: IDefaultLanguageServer | undefined,
153150
) {
154151
this.workspace = workspace || new WorkspaceService();
155152
this.workspaceRoot = workspaceFolder;
@@ -159,10 +156,9 @@ export class PythonSettings implements IPythonSettings {
159156
public static getInstance(
160157
resource: Uri | undefined,
161158
interpreterAutoSelectionService: IInterpreterAutoSelectionProxyService,
162-
workspace?: IWorkspaceService,
163-
experimentsManager?: IExperimentService,
164-
interpreterPathService?: IInterpreterPathService,
165-
defaultLS?: IDefaultLanguageServer,
159+
workspace: IWorkspaceService,
160+
interpreterPathService: IInterpreterPathService,
161+
defaultLS: IDefaultLanguageServer | undefined,
166162
): PythonSettings {
167163
workspace = workspace || new WorkspaceService();
168164
const workspaceFolderUri = PythonSettings.getSettingsUriAndTarget(resource, workspace).uri;
@@ -173,7 +169,6 @@ export class PythonSettings implements IPythonSettings {
173169
workspaceFolderUri,
174170
interpreterAutoSelectionService,
175171
workspace,
176-
experimentsManager,
177172
interpreterPathService,
178173
defaultLS,
179174
);
@@ -237,7 +232,7 @@ export class PythonSettings implements IPythonSettings {
237232
const workspaceRoot = this.workspaceRoot?.fsPath;
238233
const systemVariables: SystemVariables = new SystemVariables(undefined, workspaceRoot, this.workspace);
239234

240-
this.pythonPath = this.getPythonPath(pythonSettings, systemVariables, workspaceRoot);
235+
this.pythonPath = this.getPythonPath(systemVariables, workspaceRoot);
241236

242237
const defaultInterpreterPath = systemVariables.resolveAny(pythonSettings.get<string>('defaultInterpreterPath'));
243238
this.defaultInterpreterPath = defaultInterpreterPath || DEFAULT_INTERPRETER_SETTING;
@@ -561,24 +556,8 @@ export class PythonSettings implements IPythonSettings {
561556
this.changed.fire();
562557
}
563558

564-
private getPythonPath(
565-
pythonSettings: WorkspaceConfiguration,
566-
systemVariables: SystemVariables,
567-
workspaceRoot: string | undefined,
568-
) {
569-
/**
570-
* Note that while calling `IExperimentsManager.inExperiment()`, we assume `IExperimentsManager.activate()` is already called.
571-
* That's not true here, as this method is often called in the constructor,which runs before `.activate()` methods.
572-
* But we can still use it here for this particular experiment. Reason being that this experiment only changes
573-
* `pythonPath` setting, and I've checked that `pythonPath` setting is not accessed anywhere in the constructor.
574-
*/
575-
const inExperiment = this.experimentsManager?.inExperimentSync(DeprecatePythonPath.experiment);
576-
// Use the interpreter path service if in the experiment otherwise use the normal settings
577-
this.pythonPath = systemVariables.resolveAny(
578-
inExperiment && this.interpreterPathService
579-
? this.interpreterPathService.get(this.workspaceRoot)
580-
: pythonSettings.get<string>('pythonPath'),
581-
)!;
559+
private getPythonPath(systemVariables: SystemVariables, workspaceRoot: string | undefined) {
560+
this.pythonPath = systemVariables.resolveAny(this.interpreterPathService.get(this.workspaceRoot))!;
582561
if (
583562
!process.env.CI_DISABLE_AUTO_SELECTION &&
584563
(this.pythonPath.length === 0 || this.pythonPath === 'python') &&

0 commit comments

Comments
 (0)