Support for Jupyter to pass additional install args (#16133)

microsoft · May 4, 2021 · 23e7774 · 23e7774
1 parent 305b021
commit 23e7774
Show file tree

Hide file tree

Showing 12 changed files with 93 additions and 41 deletions.
diff --git a/news/3 Code Health/16131.md b/news/3 Code Health/16131.md
@@ -0,0 +1 @@
+Add ability for Jupyter extension to pass addtional installer arguments.
diff --git a/src/client/common/installer/condaInstaller.ts b/src/client/common/installer/condaInstaller.ts
@@ -9,7 +9,7 @@ import { inDiscoveryExperiment } from '../experiments/helpers';
 import { ExecutionInfo, IConfigurationService, IExperimentService } from '../types';
 import { isResource } from '../utils/misc';
 import { ModuleInstaller } from './moduleInstaller';
-import { InterpreterUri } from './types';
+import { InterpreterUri, ModuleInstallFlags } from './types';
 
 /**
  * A Python module installer for a conda environment.
@@ -64,7 +64,7 @@ export class CondaInstaller extends ModuleInstaller {
     protected async getExecutionInfo(
         moduleName: string,
         resource?: InterpreterUri,
-        isUpgrade?: boolean,
+        flags: ModuleInstallFlags = 0,
     ): Promise<ExecutionInfo> {
         const condaService = this.serviceContainer.get<ICondaService>(ICondaService);
         const condaFile = await condaService.getCondaFile();
@@ -77,7 +77,7 @@ export class CondaInstaller extends ModuleInstaller {
             ? this.serviceContainer.get<IComponentAdapter>(IComponentAdapter)
             : this.serviceContainer.get<ICondaLocatorService>(ICondaLocatorService);
         const info = await condaLocatorService.getCondaEnvironment(pythonPath);
-        const args = [isUpgrade ? 'update' : 'install'];
+        const args = [flags & ModuleInstallFlags.upgrade ? 'update' : 'install'];
 
         // Temporarily ensure tensorboard is installed from the conda-forge
         // channel since 2.4.1 is not yet available in the default index
@@ -93,6 +93,12 @@ export class CondaInstaller extends ModuleInstaller {
             args.push('--prefix');
             args.push(info.path.fileToCommandArgument());
         }
+        if (flags & ModuleInstallFlags.updateDependencies) {
+            args.push('--update-deps');
+        }
+        if (flags & ModuleInstallFlags.reInstall) {
+            args.push('--force-reinstall');
+        }
         args.push(moduleName);
         args.push('-y');
         return {

diff --git a/src/client/common/installer/moduleInstaller.ts b/src/client/common/installer/moduleInstaller.ts
@@ -19,7 +19,7 @@ import { ExecutionInfo, IConfigurationService, IOutputChannel, ModuleNamePurpose
 import { Products } from '../utils/localize';
 import { isResource } from '../utils/misc';
 import { ProductNames } from './productNames';
-import { IModuleInstaller, InterpreterUri } from './types';
+import { IModuleInstaller, InterpreterUri, ModuleInstallFlags } from './types';
 
 @injectable()
 export abstract class ModuleInstaller implements IModuleInstaller {
@@ -33,7 +33,7 @@ export abstract class ModuleInstaller implements IModuleInstaller {
         productOrModuleName: Product | string,
         resource?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<void> {
         const name =
             typeof productOrModuleName == 'string'
@@ -48,7 +48,7 @@ export abstract class ModuleInstaller implements IModuleInstaller {
         } else {
             options.interpreter = resource;
         }
-        const executionInfo = await this.getExecutionInfo(name, resource, isUpgrade);
+        const executionInfo = await this.getExecutionInfo(name, resource, flags);
         const terminalService = this.serviceContainer
             .get<ITerminalServiceFactory>(ITerminalServiceFactory)
             .getTerminalService(options);
@@ -132,7 +132,7 @@ export abstract class ModuleInstaller implements IModuleInstaller {
     protected abstract getExecutionInfo(
         moduleName: string,
         resource?: InterpreterUri,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<ExecutionInfo>;
     private async processInstallArgs(args: string[], resource?: InterpreterUri): Promise<string[]> {
         const indexOfPylint = args.findIndex((arg) => arg.toUpperCase() === 'PYLINT');

diff --git a/src/client/common/installer/pipEnvInstaller.ts b/src/client/common/installer/pipEnvInstaller.ts
@@ -11,7 +11,7 @@ import { inDiscoveryExperiment } from '../experiments/helpers';
 import { ExecutionInfo, IExperimentService } from '../types';
 import { isResource } from '../utils/misc';
 import { ModuleInstaller } from './moduleInstaller';
-import { InterpreterUri } from './types';
+import { InterpreterUri, ModuleInstallFlags } from './types';
 
 export const pipenvName = 'pipenv';
 
@@ -61,9 +61,14 @@ export class PipEnvInstaller extends ModuleInstaller {
     protected async getExecutionInfo(
         moduleName: string,
         _resource?: InterpreterUri,
-        isUpgrade?: boolean,
+        flags: ModuleInstallFlags = 0,
     ): Promise<ExecutionInfo> {
-        const args = [isUpgrade ? 'update' : 'install', moduleName, '--dev'];
+        // In pipenv the only way to update/upgrade or re-install is update (apart from a complete uninstall and re-install).
+        const update =
+            flags & ModuleInstallFlags.reInstall ||
+            flags & ModuleInstallFlags.updateDependencies ||
+            flags & ModuleInstallFlags.upgrade;
+        const args = [update ? 'update' : 'install', moduleName, '--dev'];
         if (moduleName === 'black') {
             args.push('--pre');
         }

diff --git a/src/client/common/installer/pipInstaller.ts b/src/client/common/installer/pipInstaller.ts
@@ -8,7 +8,7 @@ import { IPythonExecutionFactory } from '../process/types';
 import { ExecutionInfo } from '../types';
 import { isResource } from '../utils/misc';
 import { ModuleInstaller } from './moduleInstaller';
-import { InterpreterUri } from './types';
+import { InterpreterUri, ModuleInstallFlags } from './types';
 
 @injectable()
 export class PipInstaller extends ModuleInstaller {
@@ -28,16 +28,24 @@ export class PipInstaller extends ModuleInstaller {
     public isSupported(resource?: InterpreterUri): Promise<boolean> {
         return this.isPipAvailable(resource);
     }
-    protected async getExecutionInfo(moduleName: string, _resource?: InterpreterUri): Promise<ExecutionInfo> {
-        const proxyArgs: string[] = [];
+    protected async getExecutionInfo(
+        moduleName: string,
+        _resource?: InterpreterUri,
+        flags: ModuleInstallFlags = 0,
+    ): Promise<ExecutionInfo> {
+        const args: string[] = [];
         const workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
         const proxy = workspaceService.getConfiguration('http').get('proxy', '');
         if (proxy.length > 0) {
-            proxyArgs.push('--proxy');
-            proxyArgs.push(proxy);
+            args.push('--proxy');
+            args.push(proxy);
+        }
+        args.push(...['install', '-U']);
+        if (flags & ModuleInstallFlags.reInstall) {
+            args.push('--force-reinstall');
         }
         return {
-            args: [...proxyArgs, 'install', '-U', moduleName],
+            args: [...args, moduleName],
             moduleName: 'pip',
         };
     }

diff --git a/src/client/common/installer/productInstaller.ts b/src/client/common/installer/productInstaller.ts
@@ -36,6 +36,7 @@ import {
     InterpreterUri,
     IProductPathService,
     IProductService,
+    ModuleInstallFlags,
 } from './types';
 
 export { Product } from '../types';
@@ -72,7 +73,7 @@ abstract class BaseInstaller {
         product: Product,
         resource?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<InstallerResponse> {
         // If this method gets called twice, while previous promise has not been resolved, then return that same promise.
         // E.g. previous promise is not resolved as a message has been displayed to the user, so no point displaying
@@ -83,7 +84,7 @@ abstract class BaseInstaller {
         if (BaseInstaller.PromptPromises.has(key)) {
             return BaseInstaller.PromptPromises.get(key)!;
         }
-        const promise = this.promptToInstallImplementation(product, resource, cancel, isUpgrade);
+        const promise = this.promptToInstallImplementation(product, resource, cancel, flags);
         BaseInstaller.PromptPromises.set(key, promise);
         promise.then(() => BaseInstaller.PromptPromises.delete(key)).ignoreErrors();
         promise.catch(() => BaseInstaller.PromptPromises.delete(key)).ignoreErrors();
@@ -95,7 +96,7 @@ abstract class BaseInstaller {
         product: Product,
         resource?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<InstallerResponse> {
         if (product === Product.unittest) {
             return InstallerResponse.Installed;
@@ -112,7 +113,7 @@ abstract class BaseInstaller {
         }
 
         await installer
-            .installModule(product, resource, cancel, isUpgrade)
+            .installModule(product, resource, cancel, flags)
             .catch((ex) => traceError(`Error in installing the product '${ProductNames.get(product)}', ${ex}`));
 
         return this.isInstalled(product, resource).then((isInstalled) => {
@@ -207,7 +208,7 @@ abstract class BaseInstaller {
         product: Product,
         resource?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<InstallerResponse>;
 
     protected getExecutableNameFromSettings(product: Product, resource?: Uri): string {
@@ -349,7 +350,7 @@ class DataScienceInstaller extends BaseInstaller {
         product: Product,
         interpreterUri?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<InstallerResponse> {
         // Precondition
         if (isResource(interpreterUri)) {
@@ -390,7 +391,7 @@ class DataScienceInstaller extends BaseInstaller {
         }
 
         await installerModule
-            .installModule(product, interpreter, cancel, isUpgrade)
+            .installModule(product, interpreter, cancel, flags)
             .catch((ex) => traceError(`Error in installing the module '${moduleName}', ${ex}`));
 
         return this.isInstalled(product, interpreter).then((isInstalled) => {
@@ -447,15 +448,15 @@ export class ProductInstaller implements IInstaller {
         product: Product,
         resource?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<InstallerResponse> {
         const currentInterpreter = isResource(resource)
             ? await this.interpreterService.getActiveInterpreter(resource)
             : resource;
         if (!currentInterpreter) {
             return InstallerResponse.Ignore;
         }
-        return this.createInstaller(product).promptToInstall(product, resource, cancel, isUpgrade);
+        return this.createInstaller(product).promptToInstall(product, resource, cancel, flags);
     }
 
     public async isProductVersionCompatible(
@@ -470,9 +471,9 @@ export class ProductInstaller implements IInstaller {
         product: Product,
         resource?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<InstallerResponse> {
-        return this.createInstaller(product).install(product, resource, cancel, isUpgrade);
+        return this.createInstaller(product).install(product, resource, cancel, flags);
     }
 
     public async isInstalled(product: Product, resource?: InterpreterUri): Promise<boolean> {

diff --git a/src/client/common/installer/types.ts b/src/client/common/installer/types.ts
@@ -27,7 +27,7 @@ export interface IModuleInstaller {
         product: string,
         resource?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<void>;
     /**
      * Installs a Product
@@ -44,7 +44,7 @@ export interface IModuleInstaller {
         product: Product,
         resource?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<void>;
     isSupported(resource?: InterpreterUri): Promise<boolean>;
 }
@@ -76,3 +76,9 @@ export const IExtensionBuildInstaller = Symbol('IExtensionBuildInstaller');
 export interface IExtensionBuildInstaller {
     install(): Promise<void>;
 }
+
+export enum ModuleInstallFlags {
+    upgrade = 1,
+    updateDependencies = 2,
+    reInstall = 4,
+}
diff --git a/src/client/common/types.ts b/src/client/common/types.ts
@@ -21,7 +21,7 @@ import {
 import { LanguageServerType } from '../activation/types';
 import { LogLevel } from '../logging/levels';
 import type { ExtensionChannels } from './insidersBuild/types';
-import type { InterpreterUri } from './installer/types';
+import type { InterpreterUri, ModuleInstallFlags } from './installer/types';
 import { EnvironmentVariables } from './variables/types';
 import { ITestingSettings } from '../testing/configuration/types';
 
@@ -122,13 +122,13 @@ export interface IInstaller {
         product: Product,
         resource?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<InstallerResponse>;
     install(
         product: Product,
         resource?: InterpreterUri,
         cancel?: CancellationToken,
-        isUpgrade?: boolean,
+        flags?: ModuleInstallFlags,
     ): Promise<InstallerResponse>;
     isInstalled(product: Product, resource?: InterpreterUri): Promise<boolean>;
     isProductVersionCompatible(

diff --git a/src/client/jupyter/jupyterIntegration.ts b/src/client/jupyter/jupyterIntegration.ts
@@ -10,7 +10,7 @@ import { CancellationToken, Disposable, Event, Extension, Memento, Uri } from 'v
 import * as lsp from 'vscode-languageserver-protocol';
 import { ILanguageServerCache, ILanguageServerConnection } from '../activation/types';
 import { JUPYTER_EXTENSION_ID } from '../common/constants';
-import { InterpreterUri } from '../common/installer/types';
+import { InterpreterUri, ModuleInstallFlags } from '../common/installer/types';
 import {
     GLOBAL_MEMENTO,
     IExperimentService,
@@ -179,7 +179,16 @@ export class JupyterExtensionIntegration {
                 product: JupyterProductToInstall,
                 resource?: InterpreterUri,
                 cancel?: CancellationToken,
-            ): Promise<InstallerResponse> => this.installer.install(ProductMapping[product], resource, cancel),
+                reInstallAndUpdate?: boolean,
+            ): Promise<InstallerResponse> =>
+                this.installer.install(
+                    ProductMapping[product],
+                    resource,
+                    cancel,
+                    reInstallAndUpdate === true
+                        ? ModuleInstallFlags.updateDependencies | ModuleInstallFlags.reInstall
+                        : undefined,
+                ),
             getDebuggerPath: async () => dirname(getDebugpyPackagePath()),
             getInterpreterPathSelectedForJupyterServer: () =>
                 this.globalState.get<string | undefined>('INTERPRETER_PATH_SELECTED_FOR_JUPYTER_SERVER'),

diff --git a/src/client/tensorBoard/tensorBoardSession.ts b/src/client/tensorBoard/tensorBoardSession.ts
@@ -45,6 +45,7 @@ import { EventName } from '../telemetry/constants';
 import { ImportTracker } from '../telemetry/importTracker';
 import { TensorBoardPromptSelection, TensorBoardSessionStartResult } from './constants';
 import { IMultiStepInputFactory } from '../common/utils/multiStepInput';
+import { ModuleInstallFlags } from '../common/installer/types';
 
 enum Messages {
     JumpToSource = 'jump_to_source',
@@ -236,7 +237,9 @@ export class TensorBoardSession {
                     Product.tensorboard,
                     interpreter,
                     installerToken,
-                    tensorboardInstallStatus === ProductInstallStatus.NeedsUpgrade,
+                    tensorboardInstallStatus === ProductInstallStatus.NeedsUpgrade
+                        ? ModuleInstallFlags.upgrade
+                        : undefined,
                 ),
             );
         }

diff --git a/src/test/common/installer/moduleInstaller.unit.test.ts b/src/test/common/installer/moduleInstaller.unit.test.ts
@@ -27,7 +27,11 @@ import { ModuleInstaller } from '../../../client/common/installer/moduleInstalle
 import { PipEnvInstaller, pipenvName } from '../../../client/common/installer/pipEnvInstaller';
 import { PipInstaller } from '../../../client/common/installer/pipInstaller';
 import { ProductInstaller } from '../../../client/common/installer/productInstaller';
-import { IInstallationChannelManager, IModuleInstaller } from '../../../client/common/installer/types';
+import {
+    IInstallationChannelManager,
+    IModuleInstaller,
+    ModuleInstallFlags,
+} from '../../../client/common/installer/types';
 import { IFileSystem } from '../../../client/common/platform/types';
 import { ITerminalService, ITerminalServiceFactory } from '../../../client/common/terminal/types';
 import {
@@ -340,7 +344,7 @@ suite('Module Installer', () => {
                             async function installModuleAndVerifyCommand(
                                 command: string,
                                 expectedArgs: string[],
-                                isUpgrade?: boolean,
+                                flags?: ModuleInstallFlags,
                             ) {
                                 terminalService
                                     .setup((t) =>
@@ -353,7 +357,7 @@ suite('Module Installer', () => {
                                     .returns(() => Promise.resolve())
                                     .verifiable(TypeMoq.Times.once());
 
-                                await installer.installModule(product.value, resource, undefined, isUpgrade);
+                                await installer.installModule(product.value, resource, undefined, flags);
                                 terminalService.verifyAll();
                             }
 
@@ -604,7 +608,11 @@ suite('Module Installer', () => {
                                         if (moduleName === 'black') {
                                             expectedArgs.push('--pre');
                                         }
-                                        await installModuleAndVerifyCommand(pipenvName, expectedArgs, isUpgrade);
+                                        await installModuleAndVerifyCommand(
+                                            pipenvName,
+                                            expectedArgs,
+                                            isUpgrade ? ModuleInstallFlags.upgrade : undefined,
+                                        );
                                     });
                                 });
                             }
@@ -625,7 +633,11 @@ suite('Module Installer', () => {
                                         }
                                         expectedArgs.push(moduleName);
                                         expectedArgs.push('-y');
-                                        await installModuleAndVerifyCommand(condaExecutable, expectedArgs, isUpgrade);
+                                        await installModuleAndVerifyCommand(
+                                            condaExecutable,
+                                            expectedArgs,
+                                            isUpgrade ? ModuleInstallFlags.upgrade : undefined,
+                                        );
                                     });
                                 });
                             }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Add ability for Jupyter extension to pass addtional installer arguments.