Skip to content

Hide "untrusted" interpreters from 'Select interpreter' dropdown list #11047

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 3 commits into from
Apr 9, 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
1 change: 1 addition & 0 deletions news/1 Enhancements/11046.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hide "untrusted" interpreters from 'Select interpreter' dropdown list when in DeprecatePythonPath Experiment.
14 changes: 11 additions & 3 deletions src/client/interpreter/configuration/interpreterSelector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import {
IWorkspaceService
} from '../../common/application/types';
import { Commands } from '../../common/constants';
import { IConfigurationService, IPathUtils, Resource } from '../../common/types';
import { DeprecatePythonPath } from '../../common/experimentGroups';
import { IConfigurationService, IExperimentsManager, IPathUtils, Resource } from '../../common/types';
import { Interpreters } from '../../common/utils/localize';
import { IInterpreterSecurityService } from '../autoSelection/types';
import { IInterpreterService, IShebangCodeLensProvider, PythonInterpreter } from '../contracts';
import {
IInterpreterComparer,
Expand All @@ -33,7 +35,9 @@ export class InterpreterSelector implements IInterpreterSelector {
private readonly pythonPathUpdaterService: IPythonPathUpdaterServiceManager,
@inject(IShebangCodeLensProvider) private readonly shebangCodeLensProvider: IShebangCodeLensProvider,
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
@inject(ICommandManager) private readonly commandManager: ICommandManager
@inject(ICommandManager) private readonly commandManager: ICommandManager,
@inject(IExperimentsManager) private readonly experimentsManager: IExperimentsManager,
@inject(IInterpreterSecurityService) private readonly interpreterSecurityService: IInterpreterSecurityService
) {}
public dispose() {
this.disposables.forEach((disposable) => disposable.dispose());
Expand All @@ -52,7 +56,11 @@ export class InterpreterSelector implements IInterpreterSelector {
}

public async getSuggestions(resource: Resource) {
const interpreters = await this.interpreterManager.getInterpreters(resource);
let interpreters = await this.interpreterManager.getInterpreters(resource);
if (this.experimentsManager.inExperiment(DeprecatePythonPath.experiment)) {
interpreters = interpreters.filter((item) => this.interpreterSecurityService.isSafe(item) !== false);
}
this.experimentsManager.sendTelemetryIfInExperiment(DeprecatePythonPath.control);
interpreters.sort(this.interpreterComparer.compare.bind(this.interpreterComparer));
return Promise.all(interpreters.map((item) => this.suggestionToQuickPickItem(item, resource)));
}
Expand Down
41 changes: 38 additions & 3 deletions src/test/configuration/interpreterSelector.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import {
IDocumentManager,
IWorkspaceService
} from '../../client/common/application/types';
import { DeprecatePythonPath } from '../../client/common/experimentGroups';
import { PathUtils } from '../../client/common/platform/pathUtils';
import { IFileSystem } from '../../client/common/platform/types';
import { IConfigurationService, IPythonSettings } from '../../client/common/types';
import { IConfigurationService, IExperimentsManager, IPythonSettings } from '../../client/common/types';
import { Interpreters } from '../../client/common/utils/localize';
import { Architecture } from '../../client/common/utils/platform';
import { IInterpreterSecurityService } from '../../client/interpreter/autoSelection/types';
import { InterpreterSelector } from '../../client/interpreter/configuration/interpreterSelector';
import {
IInterpreterComparer,
Expand Down Expand Up @@ -66,6 +68,8 @@ suite('Interpreters - selector', () => {
let comparer: TypeMoq.IMock<IInterpreterComparer>;
let pythonPathUpdater: TypeMoq.IMock<IPythonPathUpdaterServiceManager>;
let shebangProvider: TypeMoq.IMock<IShebangCodeLensProvider>;
let experimentsManager: TypeMoq.IMock<IExperimentsManager>;
let interpreterSecurityService: TypeMoq.IMock<IInterpreterSecurityService>;
let configurationService: TypeMoq.IMock<IConfigurationService>;
let pythonSettings: TypeMoq.IMock<IPythonSettings>;
const folder1 = { name: 'one', uri: Uri.parse('one'), index: 1 };
Expand Down Expand Up @@ -96,6 +100,12 @@ suite('Interpreters - selector', () => {
let selector: TestInterpreterSelector;

setup(() => {
experimentsManager = TypeMoq.Mock.ofType<IExperimentsManager>();
experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => false);
experimentsManager
.setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control))
.returns(() => undefined);
interpreterSecurityService = TypeMoq.Mock.ofType<IInterpreterSecurityService>();
commandManager = TypeMoq.Mock.ofType<ICommandManager>();
comparer = TypeMoq.Mock.ofType<IInterpreterComparer>();
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
Expand Down Expand Up @@ -124,7 +134,9 @@ suite('Interpreters - selector', () => {
pythonPathUpdater.object,
shebangProvider.object,
configurationService.object,
commandManager.object
commandManager.object,
experimentsManager.object,
interpreterSecurityService.object
);
});

Expand All @@ -140,7 +152,9 @@ suite('Interpreters - selector', () => {
pythonPathUpdater.object,
shebangProvider.object,
configurationService.object,
commandManager.object
commandManager.object,
experimentsManager.object,
interpreterSecurityService.object
);

const initial: PythonInterpreter[] = [
Expand Down Expand Up @@ -184,6 +198,27 @@ suite('Interpreters - selector', () => {
});
});

test('When in Deprecate PythonPath experiment, remove unsafe interpreters from the suggested interpreters list', async () => {
// tslint:disable-next-line: no-any
const interpreterList = ['interpreter1', 'interpreter2', 'interpreter3'] as any;
interpreterService.setup((i) => i.getInterpreters(folder1.uri)).returns(() => interpreterList);
// tslint:disable-next-line: no-any
interpreterSecurityService.setup((i) => i.isSafe('interpreter1' as any)).returns(() => true);
// tslint:disable-next-line: no-any
interpreterSecurityService.setup((i) => i.isSafe('interpreter2' as any)).returns(() => false);
// tslint:disable-next-line: no-any
interpreterSecurityService.setup((i) => i.isSafe('interpreter3' as any)).returns(() => undefined);
experimentsManager.reset();
experimentsManager.setup((e) => e.inExperiment(DeprecatePythonPath.experiment)).returns(() => true);
experimentsManager
.setup((e) => e.sendTelemetryIfInExperiment(DeprecatePythonPath.control))
.returns(() => undefined);
// tslint:disable-next-line: no-any
selector.suggestionToQuickPickItem = (item, _) => Promise.resolve(item as any);
const suggestion = await selector.getSuggestions(folder1.uri);
assert.deepEqual(suggestion, ['interpreter1', 'interpreter3']);
});

// tslint:disable-next-line: max-func-body-length
suite('Test method setInterpreter()', async () => {
test('Update Global settings when there are no workspaces', async () => {
Expand Down
20 changes: 20 additions & 0 deletions src/test/datascience/dataScienceIocContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ import {
import { ProductService } from '../../client/common/installer/productService';
import { IInstallationChannelManager, IProductPathService, IProductService } from '../../client/common/installer/types';
import { InterpreterPathService } from '../../client/common/interpreterPathService';
import { BrowserService } from '../../client/common/net/browser';
import { IS_WINDOWS } from '../../client/common/platform/constants';
import { PathUtils } from '../../client/common/platform/pathUtils';
import { RegistryImplementation } from '../../client/common/platform/registry';
Expand Down Expand Up @@ -136,6 +137,7 @@ import {
BANNER_NAME_LS_SURVEY,
GLOBAL_MEMENTO,
IAsyncDisposableRegistry,
IBrowserService,
IConfigurationService,
ICryptoUtils,
ICurrentProcess,
Expand Down Expand Up @@ -271,6 +273,14 @@ import {
EnvironmentActivationServiceCache
} from '../../client/interpreter/activation/service';
import { IEnvironmentActivationService } from '../../client/interpreter/activation/types';
import { InterpreterEvaluation } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterEvaluation';
import { InterpreterSecurityService } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityService';
import { InterpreterSecurityStorage } from '../../client/interpreter/autoSelection/interpreterSecurity/interpreterSecurityStorage';
import {
IInterpreterEvaluation,
IInterpreterSecurityService,
IInterpreterSecurityStorage
} from '../../client/interpreter/autoSelection/types';
import { InterpreterComparer } from '../../client/interpreter/configuration/interpreterComparer';
import { InterpreterSelector } from '../../client/interpreter/configuration/interpreterSelector';
import { PythonPathUpdaterService } from '../../client/interpreter/configuration/pythonPathUpdaterService';
Expand Down Expand Up @@ -553,6 +563,16 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
this.serviceManager.addSingleton<ICodeCssGenerator>(ICodeCssGenerator, CodeCssGenerator);
this.serviceManager.addSingleton<IStatusProvider>(IStatusProvider, StatusProvider);
this.serviceManager.addSingleton<IInterpreterPathService>(IInterpreterPathService, InterpreterPathService);
this.serviceManager.addSingleton<IInterpreterSecurityService>(
IInterpreterSecurityService,
InterpreterSecurityService
);
this.serviceManager.addSingleton<IInterpreterSecurityStorage>(
IInterpreterSecurityStorage,
InterpreterSecurityStorage
);
this.serviceManager.addSingleton<IInterpreterEvaluation>(IInterpreterEvaluation, InterpreterEvaluation);
this.serviceManager.addSingleton<IBrowserService>(IBrowserService, BrowserService);
this.serviceManager.addSingletonInstance<IAsyncDisposableRegistry>(
IAsyncDisposableRegistry,
this.asyncRegistry
Expand Down