From 92db8df000ad8f665975ab08cd5c1c0c06545b7d Mon Sep 17 00:00:00 2001 From: Cory Rivera Date: Mon, 17 Aug 2020 11:02:18 -0700 Subject: [PATCH] Simplify python reinstallation by only installing jupyter. (#11820) --- .../configurePython/configurePythonWizard.ts | 2 +- .../notebookIntegration.test.ts | 6 +- .../src/jupyter/jupyterServerInstallation.ts | 108 +++--------------- 3 files changed, 21 insertions(+), 95 deletions(-) diff --git a/extensions/notebook/src/dialog/configurePython/configurePythonWizard.ts b/extensions/notebook/src/dialog/configurePython/configurePythonWizard.ts index f0b82c5b09..6689241e9c 100644 --- a/extensions/notebook/src/dialog/configurePython/configurePythonWizard.ts +++ b/extensions/notebook/src/dialog/configurePython/configurePythonWizard.ts @@ -166,7 +166,7 @@ export class ConfigurePythonWizard { let installSettings: PythonInstallSettings = { installPath: pythonLocation, existingPython: useExistingPython, - specificPackages: this.model.packagesToInstall + packages: this.model.packagesToInstall }; this.jupyterInstallation.startInstallProcess(false, installSettings) .then(() => { diff --git a/extensions/notebook/src/integrationTest/notebookIntegration.test.ts b/extensions/notebook/src/integrationTest/notebookIntegration.test.ts index 36f866cc54..62eee0df88 100644 --- a/extensions/notebook/src/integrationTest/notebookIntegration.test.ts +++ b/extensions/notebook/src/integrationTest/notebookIntegration.test.ts @@ -39,7 +39,7 @@ describe('Notebook Extension Python Installation', function () { jupyterController = notebookExtension.exports.getJupyterController() as JupyterController; console.log('Start Jupyter Installation'); - await jupyterController.jupyterInstallation.startInstallProcess(false, { installPath: pythonInstallDir, existingPython: false }); + await jupyterController.jupyterInstallation.startInstallProcess(false, { installPath: pythonInstallDir, existingPython: false, packages: [] }); installComplete = true; console.log('Jupyter Installation is done'); }); @@ -66,14 +66,14 @@ describe('Notebook Extension Python Installation', function () { console.log('Start Existing Python Installation'); let existingPythonPath = path.join(pythonInstallDir, pythonBundleVersion); - await install.startInstallProcess(false, { installPath: existingPythonPath, existingPython: true }); + await install.startInstallProcess(false, { installPath: existingPythonPath, existingPython: true, packages: [] }); should(JupyterServerInstallation.isPythonInstalled()).be.true(); should(JupyterServerInstallation.getPythonInstallPath()).be.equal(existingPythonPath); should(JupyterServerInstallation.getExistingPythonSetting()).be.true(); // Redo "new" install to restore original settings. // The actual install should get skipped since it already exists. - await install.startInstallProcess(false, { installPath: pythonInstallDir, existingPython: false }); + await install.startInstallProcess(false, { installPath: pythonInstallDir, existingPython: false, packages: [] }); should(JupyterServerInstallation.isPythonInstalled()).be.true(); should(JupyterServerInstallation.getPythonInstallPath()).be.equal(pythonInstallDir); should(JupyterServerInstallation.getExistingPythonSetting()).be.false(); diff --git a/extensions/notebook/src/jupyter/jupyterServerInstallation.ts b/extensions/notebook/src/jupyter/jupyterServerInstallation.ts index 4e7d9cdf2d..d48509f014 100644 --- a/extensions/notebook/src/jupyter/jupyterServerInstallation.ts +++ b/extensions/notebook/src/jupyter/jupyterServerInstallation.ts @@ -38,7 +38,7 @@ function msgPackageRetrievalFailed(errorMessage: string): string { return locali export interface PythonInstallSettings { installPath: string; existingPython: boolean; - specificPackages?: PythonPkgDetails[]; + packages: PythonPkgDetails[]; } export interface IJupyterServerInstallation { installCondaPackages(packages: PythonPkgDetails[], useMinVersion: boolean): Promise; @@ -74,33 +74,6 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { public static readonly DefaultPythonLocation = path.join(utils.getUserHome(), 'azuredatastudio-python'); - private readonly _commonPackages: PythonPkgDetails[] = [ - { - name: 'jupyter', - version: '1.0.0' - }, { - name: 'pandas', - version: '0.24.2' - }, { - name: 'sparkmagic', - version: '0.12.9' - } - ]; - - private readonly _commonPipPackages: PythonPkgDetails[] = [ - { - name: 'prose-codeaccelerator', - version: '1.3.0' - }, { - name: 'powershell-kernel', - version: '0.1.3' - } - ]; - - private readonly _expectedPythonPackages = this._commonPackages.concat(this._commonPipPackages); - private readonly _expectedCondaPipPackages = this._commonPipPackages; - private readonly _expectedCondaPackages: PythonPkgDetails[]; - private _kernelSetupCache: Map; private readonly _requiredKernelPackages: Map; private readonly _requiredPackagesSet: Set; @@ -124,12 +97,6 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { this._usingConda = false; this._installInProgress = false; - if (process.platform !== constants.winPlatform) { - this._expectedCondaPackages = this._commonPackages.concat([{ name: 'pykerberos', version: '1.2.1' }]); - } else { - this._expectedCondaPackages = this._commonPackages; - } - this._kernelSetupCache = new Map(); this._requiredKernelPackages = new Map(); @@ -170,7 +137,7 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { }); } - private async installDependencies(backgroundOperation: azdata.BackgroundOperation, forceInstall: boolean, specificPackages?: PythonPkgDetails[]): Promise { + private async installDependencies(backgroundOperation: azdata.BackgroundOperation, forceInstall: boolean, packages: PythonPkgDetails[]): Promise { vscode.window.showInformationMessage(msgInstallPkgStart); this.outputChannel.show(true); @@ -182,7 +149,7 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { if (!pythonExists || forceInstall) { await this.installPythonPackage(backgroundOperation, this._usingExistingPython, this._pythonInstallationPath, this.outputChannel); } - await this.upgradePythonPackages(forceInstall, specificPackages); + await this.upgradePythonPackages(forceInstall, packages); } catch (err) { this.outputChannel.appendLine(msgDependenciesInstallationFailed(utils.getErrorMessage(err))); throw err; @@ -401,7 +368,8 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { if (!installSettings) { installSettings = { installPath: this._pythonInstallationPath, - existingPython: this._usingExistingPython + existingPython: this._usingExistingPython, + packages: this.getRequiredPackagesForKernel(constants.python3DisplayName) }; } @@ -433,7 +401,7 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { description: msgTaskName, isCancelable: false, operation: op => { - this.installDependencies(op, forceInstall, installSettings.specificPackages) + this.installDependencies(op, forceInstall, installSettings.packages) .then(async () => { let notebookConfig = vscode.workspace.getConfiguration(constants.notebookConfigKey); await notebookConfig.update(constants.pythonPathConfigKey, this._pythonInstallationPath, vscode.ConfigurationTarget.Global); @@ -498,70 +466,28 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { return true; } - private async upgradePythonPackages(forceInstall: boolean, specificPackages?: PythonPkgDetails[]): Promise { - let expectedCondaPackages: PythonPkgDetails[]; - let expectedPipPackages: PythonPkgDetails[]; - if (specificPackages) { - // Always install generic packages with pip, since conda may not have them. - expectedCondaPackages = []; - expectedPipPackages = specificPackages; - } else if (this._usingConda) { - expectedCondaPackages = this._expectedCondaPackages; - expectedPipPackages = this._expectedCondaPipPackages; - } else { - expectedCondaPackages = []; - expectedPipPackages = this._expectedPythonPackages; - } - - let condaPackagesToInstall: PythonPkgDetails[]; - let pipPackagesToInstall: PythonPkgDetails[]; + private async upgradePythonPackages(forceInstall: boolean, packages: PythonPkgDetails[]): Promise { + let packagesToInstall: PythonPkgDetails[]; if (forceInstall) { - condaPackagesToInstall = expectedCondaPackages; - pipPackagesToInstall = expectedPipPackages; + packagesToInstall = packages; } else { - condaPackagesToInstall = []; - pipPackagesToInstall = []; + packagesToInstall = []; - // Conda packages - if (this._usingConda) { - let installedCondaPackages = await this.getInstalledCondaPackages(); - let condaVersionMap = new Map(); - installedCondaPackages.forEach(pkg => condaVersionMap.set(pkg.name, pkg.version)); - - expectedCondaPackages.forEach(expectedPkg => { - let installedPkgVersion = condaVersionMap.get(expectedPkg.name); - if (!installedPkgVersion || utils.comparePackageVersions(installedPkgVersion, expectedPkg.version) < 0) { - condaPackagesToInstall.push(expectedPkg); - } - }); - } - - // Pip packages let installedPipPackages = await this.getInstalledPipPackages(); let pipVersionMap = new Map(); installedPipPackages.forEach(pkg => pipVersionMap.set(pkg.name, pkg.version)); - expectedPipPackages.forEach(expectedPkg => { - let installedPkgVersion = pipVersionMap.get(expectedPkg.name); - if (!installedPkgVersion || utils.comparePackageVersions(installedPkgVersion, expectedPkg.version) < 0) { - pipPackagesToInstall.push(expectedPkg); + packages.forEach(pkg => { + let installedPkgVersion = pipVersionMap.get(pkg.name); + if (!installedPkgVersion || utils.comparePackageVersions(installedPkgVersion, pkg.version) < 0) { + packagesToInstall.push(pkg); } }); } - if (condaPackagesToInstall.length > 0 || pipPackagesToInstall.length > 0) { - let installPromise = new Promise(async (resolve, reject) => { - try { - if (this._usingConda) { - await this.installCondaPackages(condaPackagesToInstall, true); - } - await this.installPipPackages(pipPackagesToInstall, true); - resolve(); - } catch (err) { - reject(err); - } - }); - await installPromise; + if (packagesToInstall.length > 0) { + // Always install generic packages with pip, since conda may not have them (like powershell-kernel). + await this.installPipPackages(packagesToInstall, true); } }