Add discovery experiment (#14868)

* Add discovery experiment

* Add news item

* Add experiment in more places

* Tweak experiment service usage

* Revert "Tweak experiment service usage"

This reverts commit 68fdf5a.

* Revert "Add experiment in more places"

This reverts commit 5c50347.

* Tweak for tests

* Fix interpreter service unittest

* Fix linting

* Try using enable flag with experiment

* Address comments.

* Tweak hasInterpreters

Author: karthiknadig

news/1 Enhancements/14868.md

@@ -0,0 +1 @@
+Experiment to use the new environment discovery module.

package.json

@@ -1055,6 +1055,7 @@
                             "debuggerDataViewer",
                             "pythonSendEntireLineToREPL",
                             "nativeTensorBoard",
+                            "pythonDiscoveryModule",
                             "All"
                         ]
                     },
@@ -1083,6 +1084,7 @@
                             "debuggerDataViewer",
                             "pythonSendEntireLineToREPL",
                             "nativeTensorBoard",
+                            "pythonDiscoveryModule",
                             "All"
                         ]
                     },

src/client/common/experiments/groups.ts

@@ -94,3 +94,9 @@ export enum LinterInstallationPromptVariants {
     flake8First = 'pythonInstallFlake8ButtonFirst',
     noPrompt = 'pythonNotDisplayLinterPrompt'
 }
+
+// Experiment to control which environment discovery mechanism can be used
+export enum DiscoveryVariants {
+    discoverWithFileWatching = 'pythonDiscoveryModule',
+    discoveryWithoutFileWatching = 'pythonDiscoveryModuleWithoutWatcher'
+}

src/client/interpreter/interpreterService.ts

@@ -44,6 +44,7 @@ export type GetInterpreterOptions = {
 // The parts of IComponentAdapter used here.
 interface IComponent {
     getInterpreterDetails(pythonPath: string): Promise<undefined | PythonEnvironment>;
+    getInterpreters(resource?: Uri): Promise<PythonEnvironment[] | undefined>;
 }
 
 @injectable()
@@ -68,7 +69,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
     private readonly persistentStateFactory: IPersistentStateFactory;
     private readonly configService: IConfigurationService;
     private readonly interpreterPathService: IInterpreterPathService;
-    private readonly experiments: IExperimentsManager;
+    private readonly experimentsManager: IExperimentsManager;
     private readonly didChangeInterpreterEmitter = new EventEmitter<void>();
     private readonly didChangeInterpreterInformation = new EventEmitter<PythonEnvironment>();
     private readonly inMemoryCacheOfDisplayNames = new Map<string, string>();
@@ -86,7 +87,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
         this.persistentStateFactory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
         this.configService = this.serviceContainer.get<IConfigurationService>(IConfigurationService);
         this.interpreterPathService = this.serviceContainer.get<IInterpreterPathService>(IInterpreterPathService);
-        this.experiments = this.serviceContainer.get<IExperimentsManager>(IExperimentsManager);
+        this.experimentsManager = this.serviceContainer.get<IExperimentsManager>(IExperimentsManager);
     }
 
     public async refresh(resource?: Uri) {
@@ -105,7 +106,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
         const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
         const pySettings = this.configService.getSettings();
         this._pythonPathSetting = pySettings.pythonPath;
-        if (this.experiments.inExperiment(DeprecatePythonPath.experiment)) {
+        if (this.experimentsManager.inExperiment(DeprecatePythonPath.experiment)) {
             disposables.push(
                 this.interpreterPathService.onDidChange((i) => {
                     this._onConfigChanged(i.uri);
@@ -124,14 +125,22 @@ export class InterpreterService implements Disposable, IInterpreterService {
             });
             disposables.push(disposable);
         }
-        this.experiments.sendTelemetryIfInExperiment(DeprecatePythonPath.control);
+        this.experimentsManager.sendTelemetryIfInExperiment(DeprecatePythonPath.control);
     }
 
     @captureTelemetry(EventName.PYTHON_INTERPRETER_DISCOVERY, { locator: 'all' }, true)
     public async getInterpreters(resource?: Uri, options?: GetInterpreterOptions): Promise<PythonEnvironment[]> {
-        const interpreters = await this.locator.getInterpreters(resource, options);
+        let environments: PythonEnvironment[] = [];
+
+        const envs = await this.pyenvs.getInterpreters(resource);
+        if (envs !== undefined) {
+            environments = envs;
+        } else {
+            environments = await this.locator.getInterpreters(resource, options);
+        }
+
         await Promise.all(
-            interpreters
+            environments
                 .filter((item) => !item.displayName)
                 .map(async (item) => {
                     item.displayName = await this.getDisplayName(item, resource);
@@ -141,7 +150,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
                     }
                 })
         );
-        return interpreters;
+        return environments;
     }
 
     public dispose(): void {

src/client/pythonEnvironments/common/externalDependencies.ts

@@ -5,6 +5,7 @@ import * as fsapi from 'fs-extra';
 import * as path from 'path';
 import * as vscode from 'vscode';
 import { ExecutionResult, IProcessServiceFactory, SpawnOptions } from '../../common/process/types';
+import { IExperimentService } from '../../common/types';
 import { chain, iterable } from '../../common/utils/async';
 import { getOSType, OSType } from '../../common/utils/platform';
 import { IDisposable } from '../../common/utils/resourceLifecycle';
@@ -114,3 +115,8 @@ export function onDidChangePythonSetting(name: string, callback: () => void): ID
         }
     });
 }
+
+export function inExperiment(experiment: string): Promise<boolean> {
+    const experimentService = internalServiceContainer.get<IExperimentService>(IExperimentService);
+    return experimentService.inExperiment(experiment);
+}

src/client/pythonEnvironments/legacyIOC.ts

@@ -3,6 +3,7 @@
 
 import { injectable } from 'inversify';
 import * as vscode from 'vscode';
+import { DiscoveryVariants } from '../common/experiments/groups';
 import { getVersionString, parseVersion } from '../common/utils/version';
 import {
     CONDA_ENV_FILE_SERVICE,
@@ -31,6 +32,7 @@ import { PythonEnvInfo, PythonEnvKind, PythonReleaseLevel } from './base/info';
 import { buildEnvInfo } from './base/info/env';
 import { ILocator, PythonLocatorQuery } from './base/locator';
 import { getEnvs } from './base/locatorUtils';
+import { inExperiment } from './common/externalDependencies';
 import { PythonInterpreterLocatorService } from './discovery/locators';
 import { InterpreterLocatorHelper } from './discovery/locators/helpers';
 import { InterpreterLocatorProgressService } from './discovery/locators/progressService';
@@ -134,19 +136,20 @@ export interface IPythonEnvironments extends ILocator {}
 
 @injectable()
 class ComponentAdapter implements IComponentAdapter {
+    // this will be set based on experiment
+    private _enabled?: boolean;
+
     constructor(
         // The adapter only wraps one thing: the component API.
         private readonly api: IPythonEnvironments,
         private readonly environmentsSecurity: IEnvironmentsSecurity,
-        // For now we effectively disable the component.
-        private readonly enabled = false,
     ) {}
 
     // IInterpreterHelper
 
     // A result of `undefined` means "Fall back to the old code!"
     public async getInterpreterInformation(pythonPath: string): Promise<undefined | Partial<PythonEnvironment>> {
-        if (!this.enabled) {
+        if (!(await this.isEnabled())) {
             return undefined;
         }
         const env = await this.api.resolveEnv(pythonPath);
@@ -158,7 +161,7 @@ class ComponentAdapter implements IComponentAdapter {
 
     // A result of `undefined` means "Fall back to the old code!"
     public async isMacDefaultPythonPath(pythonPath: string): Promise<boolean | undefined> {
-        if (!this.enabled) {
+        if (!(await this.isEnabled())) {
             return undefined;
         }
         const env = await this.api.resolveEnv(pythonPath);
@@ -177,7 +180,7 @@ class ComponentAdapter implements IComponentAdapter {
         pythonPath: string,
         resource?: vscode.Uri,
     ): Promise<undefined | PythonEnvironment> {
-        if (!this.enabled) {
+        if (!(await this.isEnabled())) {
             return undefined;
         }
         const info = buildEnvInfo({ executable: pythonPath });
@@ -198,7 +201,7 @@ class ComponentAdapter implements IComponentAdapter {
 
     // A result of `undefined` means "Fall back to the old code!"
     public async isCondaEnvironment(interpreterPath: string): Promise<boolean | undefined> {
-        if (!this.enabled) {
+        if (!(await this.isEnabled())) {
             return undefined;
         }
         const env = await this.api.resolveEnv(interpreterPath);
@@ -210,7 +213,7 @@ class ComponentAdapter implements IComponentAdapter {
 
     // A result of `undefined` means "Fall back to the old code!"
     public async getCondaEnvironment(interpreterPath: string): Promise<CondaEnvironmentInfo | undefined> {
-        if (!this.enabled) {
+        if (!(await this.isEnabled())) {
             return undefined;
         }
         const env = await this.api.resolveEnv(interpreterPath);
@@ -231,25 +234,27 @@ class ComponentAdapter implements IComponentAdapter {
 
     // A result of `undefined` means "Fall back to the old code!"
     public async isWindowsStoreInterpreter(pythonPath: string): Promise<boolean | undefined> {
-        if (!this.enabled) {
+        if (!(await this.isEnabled())) {
             return undefined;
         }
         const env = await this.api.resolveEnv(pythonPath);
-        if (env === undefined) {
-            return undefined;
+        if (env) {
+            return env.kind === PythonEnvKind.WindowsStore;
         }
-        return env.kind === PythonEnvKind.WindowsStore;
+        return undefined;
     }
 
     // IInterpreterLocatorService
 
     // A result of `undefined` means "Fall back to the old code!"
     public get hasInterpreters(): Promise<boolean | undefined> {
-        if (!this.enabled) {
-            return Promise.resolve(undefined);
-        }
-        const iterator = this.api.iterEnvs();
-        return iterator.next().then((res) => !res.done);
+        return this.isEnabled().then((enabled) => {
+            if (enabled) {
+                const iterator = this.api.iterEnvs();
+                return iterator.next().then((res) => !res.done);
+            }
+            return undefined;
+        });
     }
 
     // A result of `undefined` means "Fall back to the old code!"
@@ -262,7 +267,7 @@ class ComponentAdapter implements IComponentAdapter {
         //     onSuggestion?: boolean;
         // }
     ): Promise<PythonEnvironment[] | undefined> {
-        if (!this.enabled) {
+        if (!(await this.isEnabled())) {
             return undefined;
         }
         if (options?.onSuggestion) {
@@ -285,6 +290,19 @@ class ComponentAdapter implements IComponentAdapter {
         const envs = await getEnvs(iterator);
         return envs.map(convertEnvInfo);
     }
+
+    private async isEnabled(): Promise<boolean> {
+        if (this._enabled === undefined) {
+            this._enabled = (await Promise.all(
+                [
+                    inExperiment(DiscoveryVariants.discoverWithFileWatching),
+                    inExperiment(DiscoveryVariants.discoveryWithoutFileWatching),
+                ],
+            )).includes(true);
+        }
+
+        return this._enabled;
+    }
 }
 
 export function registerLegacyDiscoveryForIOC(

src/test/interpreters/interpreterService.unit.test.ts

@@ -21,6 +21,7 @@ import { IPythonExecutionFactory, IPythonExecutionService } from '../../client/c
 import {
     IConfigurationService,
     IDisposableRegistry,
+    IExperimentService,
     IExperimentsManager,
     IInterpreterPathService,
     InterpreterConfigurationScope,
@@ -72,6 +73,7 @@ suite('Interpreters service', () => {
     let configService: TypeMoq.IMock<IConfigurationService>;
     let interpreterPathService: TypeMoq.IMock<IInterpreterPathService>;
     let experimentsManager: TypeMoq.IMock<IExperimentsManager>;
+    let experimentService: TypeMoq.IMock<IExperimentService>;
     let pythonSettings: TypeMoq.IMock<IPythonSettings>;
     let hashProviderFactory: TypeMoq.IMock<IInterpreterHashProviderFactory>;
 
@@ -81,6 +83,7 @@ suite('Interpreters service', () => {
         serviceContainer = new ServiceContainer(cont);
 
         experimentsManager = TypeMoq.Mock.ofType<IExperimentsManager>();
+        experimentService = TypeMoq.Mock.ofType<IExperimentService>();
         interpreterPathService = TypeMoq.Mock.ofType<IInterpreterPathService>();
         updater = TypeMoq.Mock.ofType<IPythonPathUpdaterServiceManager>();
         pyenvs = TypeMoq.Mock.ofType<IComponentAdapter>();
@@ -130,6 +133,7 @@ suite('Interpreters service', () => {
         );
         serviceManager.addSingletonInstance<IFileSystem>(IFileSystem, fileSystem.object);
         serviceManager.addSingletonInstance<IExperimentsManager>(IExperimentsManager, experimentsManager.object);
+        serviceManager.addSingletonInstance<IExperimentService>(IExperimentService, experimentService.object);
         serviceManager.addSingletonInstance<IInterpreterPathService>(
             IInterpreterPathService,
             interpreterPathService.object