Skip to content
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

Deprecate PythonPath - Part 1 #11011

Merged
merged 7 commits into from
Apr 8, 2020
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
13 changes: 12 additions & 1 deletion experiments.json
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,17 @@
"salt": "EnableIPyWidgets",
"min": 0,
"max": 0
},
{
"name": "DeprecatePythonPath - experiment",
"salt": "DeprecatePythonPath",
"min": 0,
"max": 0
},
{
"name": "DeprecatePythonPath - control",
"salt": "DeprecatePythonPath",
"min": 100,
"max": 100
}

]
1 change: 1 addition & 0 deletions news/1 Enhancements/10325.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use new interpreter storage supporting multiroot workspaces when in Deprecate PythonPath experiment.
1 change: 1 addition & 0 deletions news/1 Enhancements/10372.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Modified `Select interpreter` command to support setting interpreter at workspace level.
1 change: 1 addition & 0 deletions news/1 Enhancements/10374.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added a command `Clear Workspace Interpreter Setting` to clear value of Python interpreter from workspace settings.
1 change: 1 addition & 0 deletions news/1 Enhancements/10879.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Prompt when an "untrusted" workspace Python environment is to be auto selected when in Deprecate PythonPath experiment.
1 change: 1 addition & 0 deletions news/1 Enhancements/10912.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added a command `Reset stored info for untrusted Interpreters` to reset "untrusted" interpreters storage when in Deprecate PythonPath experiment.
1 change: 1 addition & 0 deletions news/1 Enhancements/11021.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added a user setting `python.defaultInterpreterPath` to set up the default interpreter path when in Deprecate PythonPath experiment.
30 changes: 30 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@
"onCommand:python.switchOffInsidersChannel",
"onCommand:python.switchToDailyChannel",
"onCommand:python.switchToWeeklyChannel",
"onCommand:python.clearWorkspaceInterpreter",
"onCommand:python.resetInterpreterSecurityStorage",
"onCommand:python.datascience.createnewnotebook",
"onCommand:python.datascience.showhistorypane",
"onCommand:python.datascience.importnotebook",
Expand Down Expand Up @@ -224,6 +226,16 @@
"title": "%python.command.python.switchToWeeklyChannel.title%",
"category": "Python"
},
{
"command": "python.clearWorkspaceInterpreter",
"title": "%python.command.python.clearWorkspaceInterpreter.title%",
"category": "Python"
},
{
"command": "python.resetInterpreterSecurityStorage",
"title": "%python.command.python.resetInterpreterSecurityStorage.title%",
"category": "Python"
},
{
"command": "python.refactorExtractVariable",
"title": "%python.command.python.refactorExtractVariable.title%",
Expand Down Expand Up @@ -753,6 +765,16 @@
"category": "Python",
"when": "config.python.insidersChannel != 'weekly'"
},
{
"command": "python.clearWorkspaceInterpreter",
"title": "%python.command.python.clearWorkspaceInterpreter.title%",
"category": "Python"
},
{
"command": "python.resetInterpreterSecurityStorage",
"title": "%python.command.python.resetInterpreterSecurityStorage.title%",
"category": "Python"
},
{
"command": "python.viewOutput",
"title": "%python.command.python.viewOutput.title%",
Expand Down Expand Up @@ -1554,6 +1576,12 @@
"description": "Enables/disables A/B tests.",
"scope": "machine"
},
"python.defaultInterpreterPath": {
"type": "string",
"default": "python",
"description": "Path to Python, you can use a custom version of Python by modifying this setting to include the full path.",
"scope": "machine"
},
"python.experiments.optInto": {
"type": "array",
"default": [],
Expand All @@ -1571,6 +1599,7 @@
"UseTerminalToGetActivatedEnvVars - experiment",
"CollectLSRequestTiming - experiment",
"CollectNodeLSRequestTiming - experiment",
"DeprecatePythonPath - experiment",
"All"
]
},
Expand All @@ -1594,6 +1623,7 @@
"UseTerminalToGetActivatedEnvVars - experiment",
"CollectLSRequestTiming - experiment",
"CollectNodeLSRequestTiming - experiment",
"DeprecatePythonPath - experiment",
"All"
]
},
Expand Down
10 changes: 8 additions & 2 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
"python.command.python.switchOffInsidersChannel.title": "Switch to Default Channel",
"python.command.python.switchToDailyChannel.title": "Switch to Insiders Daily Channel",
"python.command.python.switchToWeeklyChannel.title": "Switch to Insiders Weekly Channel",
"python.command.python.clearWorkspaceInterpreter.title": "Clear Workspace Interpreter Setting",
"python.command.python.resetInterpreterSecurityStorage.title": "Reset Stored Info for Untrusted Interpreters",
"python.command.python.refactorExtractVariable.title": "Extract Variable",
"python.command.python.refactorExtractMethod.title": "Extract Method",
"python.command.python.viewOutput.title": "Show Output",
Expand Down Expand Up @@ -131,14 +133,20 @@
"DataScience.connectingToJupyter": "Connecting to Jupyter server",
"Experiments.inGroup": "User belongs to experiment group '{0}'",
"Interpreters.RefreshingInterpreters": "Refreshing Python Interpreters",
"Interpreters.entireWorkspace": "Entire workspace",
"Interpreters.pythonInterpreterPath": "Python interpreter path: {0}",
"Interpreters.LoadingInterpreters": "Loading Python Interpreters",
"Interpreters.unsafeInterpreterMessage": "We found a Python environment in this workspace. Do you want to select it to start up the features in the Python extension? Only accept if you trust this environment.",
"Interpreters.condaInheritEnvMessage": "We noticed you're using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change \"terminal.integrated.inheritEnv\" to false in your user settings.",
"Logging.CurrentWorkingDirectory": "cwd:",
"Common.bannerLabelYes": "Yes",
"Common.bannerLabelNo": "No",
"Common.doNotShowAgain": "Do not show again",
"Common.reload": "Reload",
"Common.moreInfo": "More Info",
"Common.and": "and",
"Common.install": "Install",
"Common.learnMore": "Learn more",
"OutputChannelNames.languageServer": "Python Language Server",
"OutputChannelNames.python": "Python",
"OutputChannelNames.pythonTest": "Python Test Log",
Expand Down Expand Up @@ -166,8 +174,6 @@
"DataScienceSurveyBanner.bannerLabelYes": "Yes, take survey now",
"DataScienceSurveyBanner.bannerLabelNo": "No, thanks",
"InteractiveShiftEnterBanner.bannerMessage": "Would you like to run code in the 'Python Interactive' window (an IPython console) for 'shift-enter'? Select 'No' to continue to run code in the Python Terminal. This can be changed later in settings.",
"InteractiveShiftEnterBanner.bannerLabelYes": "Yes",
"InteractiveShiftEnterBanner.bannerLabelNo": "No",
"DataScience.restartingKernelStatus": "Restarting IPython Kernel",
"DataScience.executingCode": "Executing Cell",
"DataScience.collapseAll": "Collapse all cell inputs",
Expand Down
45 changes: 41 additions & 4 deletions src/client/activation/activationManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,21 @@ import { inject, injectable, multiInject } from 'inversify';
import { TextDocument } from 'vscode';
import { IApplicationDiagnostics } from '../application/types';
import { IActiveResourceService, IDocumentManager, IWorkspaceService } from '../common/application/types';
import { PYTHON_LANGUAGE } from '../common/constants';
import { DEFAULT_INTERPRETER_SETTING, PYTHON_LANGUAGE } from '../common/constants';
import { DeprecatePythonPath } from '../common/experimentGroups';
import { traceDecorators } from '../common/logger';
import { IFileSystem } from '../common/platform/types';
import { IDisposable, Resource } from '../common/types';
import { IInterpreterAutoSelectionService } from '../interpreter/autoSelection/types';
import { IDisposable, IExperimentsManager, IInterpreterPathService, Resource } from '../common/types';
import { createDeferred, Deferred } from '../common/utils/async';
import { IInterpreterAutoSelectionService, IInterpreterSecurityService } from '../interpreter/autoSelection/types';
import { IInterpreterService } from '../interpreter/contracts';
import { sendActivationTelemetry } from '../telemetry/envFileTelemetry';
import { IExtensionActivationManager, IExtensionActivationService, IExtensionSingleActivationService } from './types';

@injectable()
export class ExtensionActivationManager implements IExtensionActivationManager {
public readonly activatedWorkspaces = new Set<string>();
protected readonly isInterpreterSetForWorkspacePromises = new Map<string, Deferred<void>>();
private readonly disposables: IDisposable[] = [];
private docOpenedHandler?: IDisposable;
constructor(
Expand All @@ -31,7 +34,10 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
@inject(IApplicationDiagnostics) private readonly appDiagnostics: IApplicationDiagnostics,
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
@inject(IFileSystem) private readonly fileSystem: IFileSystem,
@inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService
@inject(IActiveResourceService) private readonly activeResourceService: IActiveResourceService,
@inject(IExperimentsManager) private readonly experiments: IExperimentsManager,
@inject(IInterpreterPathService) private readonly interpreterPathService: IInterpreterPathService,
@inject(IInterpreterSecurityService) private readonly interpreterSecurityService: IInterpreterSecurityService
) {}

public dispose() {
Expand Down Expand Up @@ -66,6 +72,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
await sendActivationTelemetry(this.fileSystem, this.workspaceService, resource);

await this.autoSelection.autoSelectInterpreter(resource);
await this.evaluateAutoSelectedInterpreterSafety(resource);
await Promise.all(this.activationServices.map((item) => item.activate(resource)));
await this.appDiagnostics.performPreStartupHealthCheck(resource);
}
Expand All @@ -88,8 +95,38 @@ export class ExtensionActivationManager implements IExtensionActivationManager {
const folder = this.workspaceService.getWorkspaceFolder(doc.uri);
this.activateWorkspace(folder ? folder.uri : undefined).ignoreErrors();
}

public async evaluateAutoSelectedInterpreterSafety(resource: Resource) {
if (this.experiments.inExperiment(DeprecatePythonPath.experiment)) {
const workspaceKey = this.getWorkspaceKey(resource);
const interpreterSettingValue = this.interpreterPathService.get(resource);
if (interpreterSettingValue.length === 0 || interpreterSettingValue === DEFAULT_INTERPRETER_SETTING) {
// Setting is not set, extension will use the autoselected value. Make sure it's safe.
const interpreter = this.autoSelection.getAutoSelectedInterpreter(resource);
if (interpreter) {
const isInterpreterSetForWorkspace = createDeferred<void>();
this.isInterpreterSetForWorkspacePromises.set(workspaceKey, isInterpreterSetForWorkspace);
await Promise.race([
isInterpreterSetForWorkspace.promise,
this.interpreterSecurityService.evaluateAndRecordInterpreterSafety(interpreter, resource)
]);
}
} else {
// Resolve any concurrent calls waiting on the promise
if (this.isInterpreterSetForWorkspacePromises.has(workspaceKey)) {
this.isInterpreterSetForWorkspacePromises.get(workspaceKey)!.resolve();
this.isInterpreterSetForWorkspacePromises.delete(workspaceKey);
}
}
}
this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control);
}

protected addHandlers() {
this.disposables.push(this.workspaceService.onDidChangeWorkspaceFolders(this.onWorkspaceFoldersChanged, this));
this.disposables.push(
this.interpreterPathService.onDidChange((i) => this.evaluateAutoSelectedInterpreterSafety(i.uri))
);
}
protected addRemoveDocOpenedHandlers() {
if (this.hasMultipleWorkspaces()) {
Expand Down
4 changes: 1 addition & 3 deletions src/client/activation/serviceRegistry.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { ActiveResourceService } from '../common/application/activeResource';
import { IActiveResourceService } from '../common/application/types';

import { registerTypes as registerDotNetTypes } from '../common/dotnet/serviceRegistry';
import { INugetRepository } from '../common/nuget/types';
import {
Expand Down Expand Up @@ -211,6 +210,5 @@ export function registerTypes(serviceManager: IServiceManager, languageServerTyp
IExtensionSingleActivationService,
ExtensionSurveyPrompt
);
serviceManager.addSingleton<IActiveResourceService>(IActiveResourceService, ActiveResourceService);
serviceManager.addSingleton<IExtensionSingleActivationService>(IExtensionSingleActivationService, AATesting);
}
51 changes: 39 additions & 12 deletions src/client/application/diagnostics/checks/macPythonInterpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,17 @@
import { inject, injectable } from 'inversify';
import { ConfigurationChangeEvent, DiagnosticSeverity, Uri } from 'vscode';
import { IWorkspaceService } from '../../../common/application/types';
import { DeprecatePythonPath } from '../../../common/experimentGroups';
import '../../../common/extensions';
import { IPlatformService } from '../../../common/platform/types';
import { IConfigurationService, IDisposableRegistry, Resource } from '../../../common/types';
import {
IConfigurationService,
IDisposableRegistry,
IExperimentsManager,
IInterpreterPathService,
InterpreterConfigurationScope,
Resource
} from '../../../common/types';
import { IInterpreterHelper, IInterpreterService, InterpreterType } from '../../../interpreter/contracts';
import { IServiceContainer } from '../../../ioc/types';
import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
Expand Down Expand Up @@ -133,18 +141,37 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
protected addPythonPathChangedHandler() {
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
const disposables = this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
const interpreterPathService = this.serviceContainer.get<IInterpreterPathService>(IInterpreterPathService);
const experiments = this.serviceContainer.get<IExperimentsManager>(IExperimentsManager);
if (experiments.inExperiment(DeprecatePythonPath.experiment)) {
disposables.push(interpreterPathService.onDidChange((i) => this.onDidChangeConfiguration(undefined, i)));
}
experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control);
disposables.push(workspaceService.onDidChangeConfiguration(this.onDidChangeConfiguration.bind(this)));
}
protected async onDidChangeConfiguration(event: ConfigurationChangeEvent) {
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
const workspacesUris: (Uri | undefined)[] = workspaceService.hasWorkspaceFolders
? workspaceService.workspaceFolders!.map((workspace) => workspace.uri)
: [undefined];
const workspaceUriIndex = workspacesUris.findIndex((uri) =>
event.affectsConfiguration('python.pythonPath', uri)
);
if (workspaceUriIndex === -1) {
return;
protected async onDidChangeConfiguration(
event?: ConfigurationChangeEvent,
interpreterConfigurationScope?: InterpreterConfigurationScope
) {
let workspaceUri: Resource;
if (event) {
const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
const workspacesUris: (Uri | undefined)[] = workspaceService.hasWorkspaceFolders
? workspaceService.workspaceFolders!.map((workspace) => workspace.uri)
: [undefined];
const workspaceUriIndex = workspacesUris.findIndex((uri) =>
event.affectsConfiguration('python.pythonPath', uri)
);
if (workspaceUriIndex === -1) {
return;
}
workspaceUri = workspacesUris[workspaceUriIndex];
} else if (interpreterConfigurationScope) {
workspaceUri = interpreterConfigurationScope.uri;
} else {
throw new Error(
'One of `interpreterConfigurationScope` or `event` should be defined when calling `onDidChangeConfiguration`.'
);
}
// Lets wait, for more changes, dirty simple throttling.
if (this.timeOut) {
Expand All @@ -154,7 +181,7 @@ export class InvalidMacPythonInterpreterService extends BaseDiagnosticsService {
}
this.timeOut = setTimeout(() => {
this.timeOut = undefined;
this.diagnose(workspacesUris[workspaceUriIndex])
this.diagnose(workspaceUri)
.then((diagnostics) => this.handle(diagnostics))
.ignoreErrors();
}, this.changeThrottleTimeout);
Expand Down
2 changes: 2 additions & 0 deletions src/client/common/application/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ export type CommandsWithoutArgs = keyof ICommandNameWithoutArgumentTypeMapping;
interface ICommandNameWithoutArgumentTypeMapping {
[Commands.SwitchToInsidersDaily]: [];
[Commands.SwitchToInsidersWeekly]: [];
[Commands.ClearWorkspaceInterpreter]: [];
[Commands.ResetInterpreterSecurityStorage]: [];
[Commands.SwitchOffInsidersChannel]: [];
[Commands.Set_Interpreter]: [];
[Commands.Set_ShebangInterpreter]: [];
Expand Down
31 changes: 31 additions & 0 deletions src/client/common/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,37 @@ export interface IWorkspaceService {
*/
readonly workspaceFolders: readonly WorkspaceFolder[] | undefined;

/**
* The location of the workspace file, for example:
*
* `file:///Users/name/Development/myProject.code-workspace`
*
* or
*
* `untitled:1555503116870`
*
* for a workspace that is untitled and not yet saved.
*
* Depending on the workspace that is opened, the value will be:
* * `undefined` when no workspace or a single folder is opened
* * the path of the workspace file as `Uri` otherwise. if the workspace
* is untitled, the returned URI will use the `untitled:` scheme
*
* The location can e.g. be used with the `vscode.openFolder` command to
* open the workspace again after it has been closed.
*
* **Example:**
* ```typescript
* vscode.commands.executeCommand('vscode.openFolder', uriOfWorkspace);
* ```
*
* **Note:** it is not advised to use `workspace.workspaceFile` to write
* configuration data into the file. You can use `workspace.getConfiguration().update()`
* for that purpose which will work both when a single folder is opened as
* well as an untitled or saved workspace.
*/
readonly workspaceFile: Resource;

/**
* An event that is emitted when a workspace folder is added or removed.
*/
Expand Down
Loading