diff --git a/extensions/notebook/package.json b/extensions/notebook/package.json index 58fe73f396..badccfb419 100644 --- a/extensions/notebook/package.json +++ b/extensions/notebook/package.json @@ -370,7 +370,7 @@ }, { "command": "jupyter.cmd.managePackages", - "when": "false" + "when": "notebook:pythonInstalled && !notebook:runningOnSAW" }, { "command": "notebook.command.saveBook", diff --git a/extensions/notebook/package.nls.json b/extensions/notebook/package.nls.json index c010198e92..d1ae576f38 100644 --- a/extensions/notebook/package.nls.json +++ b/extensions/notebook/package.nls.json @@ -38,7 +38,7 @@ "config.jupyter.kernelConfigValuesDescription": "Configuration options for Jupyter kernels. This is automatically managed and not recommended to be manually edited.", "title.reinstallNotebookDependencies": "Reinstall Notebook dependencies", "title.configurePython": "Configure Python for Notebooks", - "title.managePackages": "Manage Packages", + "title.managePackages": "Manage Notebook Packages", "title.SQL19PreviewBook": "SQL Server 2019 Guide", "books-preview-category": "Jupyter Books", "title.saveJupyterBook": "Save Jupyter Book", diff --git a/extensions/notebook/src/dialog/managePackages/managePackagesDialog.ts b/extensions/notebook/src/dialog/managePackages/managePackagesDialog.ts index 51ac318396..a4ffe4a1c9 100644 --- a/extensions/notebook/src/dialog/managePackages/managePackagesDialog.ts +++ b/extensions/notebook/src/dialog/managePackages/managePackagesDialog.ts @@ -27,7 +27,7 @@ export class ManagePackagesDialog { * Opens a dialog to manage packages used by notebooks. */ public showDialog(): void { - this.dialog = azdata.window.createModelViewDialog(localize('managePackages.dialogName', "Manage Packages")); + this.dialog = azdata.window.createModelViewDialog(localize('managePackages.dialogName', "Manage Notebook Packages")); this.installedPkgTab = new InstalledPackagesTab(this, this.jupyterInstallation); this.addNewPkgTab = new AddNewPackageTab(this, this.jupyterInstallation); diff --git a/extensions/notebook/src/extension.ts b/extensions/notebook/src/extension.ts index 85cf66db79..f5e508992b 100644 --- a/extensions/notebook/src/extension.ts +++ b/extensions/notebook/src/extension.ts @@ -12,7 +12,7 @@ import { AppContext } from './common/appContext'; import { IExtensionApi, IPackageManageProvider } from './types'; import { CellType } from './contracts/content'; import { NotebookUriHandler } from './protocol/notebookUriHandler'; -import { BuiltInCommands, unsavedBooksContextKey } from './common/constants'; +import { BuiltInCommands, CommandContext, unsavedBooksContextKey } from './common/constants'; import { RemoteBookController } from './book/remoteBookController'; import { RemoteBookDialog } from './dialog/remoteBookDialog'; import { RemoteBookDialogModel } from './dialog/remoteBookDialogModel'; @@ -22,6 +22,7 @@ import { BookTreeItem } from './book/bookTreeItem'; import Logger from './common/logger'; import { sendNotebookActionEvent, NbTelemetryView, NbTelemetryAction } from './telemetry'; import { TelemetryReporter } from './telemetry'; +import { JupyterServerInstallation } from './jupyter/jupyterServerInstallation'; const localize = nls.loadMessageBundle(); @@ -39,6 +40,11 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi if (vscode.env.uiKind === vscode.UIKind.Web) { await config.update('allowRoot', true, vscode.ConfigurationTarget.Global); } + + // Check if python is already installed in order to enable the Manage Notebook Packages dialog + let pythonInstalled = JupyterServerInstallation.isPythonInstalled(); + await vscode.commands.executeCommand(BuiltInCommands.SetContext, CommandContext.NotebookPythonInstalled, pythonInstalled); + /** * ***** IMPORTANT ***** * If changes are made to bookTreeView.openBook, please ensure backwards compatibility with its current state. @@ -154,7 +160,6 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi return undefined; } - const bookTreeViewProvider = appContext.bookTreeViewProvider; await bookTreeViewProvider.initialized; const providedBookTreeViewProvider = appContext.providedBookTreeViewProvider; diff --git a/extensions/notebook/src/jupyter/jupyterController.ts b/extensions/notebook/src/jupyter/jupyterController.ts index 237b9f2176..bb6dae914a 100644 --- a/extensions/notebook/src/jupyter/jupyterController.ts +++ b/extensions/notebook/src/jupyter/jupyterController.ts @@ -155,21 +155,26 @@ export class JupyterController { } public async doManagePackages(options?: ManagePackageDialogOptions | vscode.Uri): Promise { - try { - if (!options || options instanceof vscode.Uri) { - options = { - defaultLocation: constants.localhostName, - defaultProviderId: LocalPipPackageManageProvider.ProviderId - }; - } - let model = new ManagePackagesDialogModel(this._jupyterInstallation, this._packageManageProviders, options); + // Handle the edge case where python is installed and then deleted manually from the user settings. + if (!JupyterServerInstallation.isPythonInstalled()) { + await vscode.window.showErrorMessage(localize('pythonNotSetup', "Python is not currently configured for notebooks. The 'Configure Python for Notebooks' command must be run before being able to manage notebook packages.")); + } else { + try { + if (!options || options instanceof vscode.Uri) { + options = { + defaultLocation: constants.localhostName, + defaultProviderId: LocalPipPackageManageProvider.ProviderId + }; + } + let model = new ManagePackagesDialogModel(this._jupyterInstallation, this._packageManageProviders, options); - await model.init(); - let packagesDialog = new ManagePackagesDialog(model, this.extensionContext); - packagesDialog.showDialog(); - } catch (error) { - let message = utils.getErrorMessage(error); - void vscode.window.showErrorMessage(message); + await model.init(); + let packagesDialog = new ManagePackagesDialog(model, this.extensionContext); + packagesDialog.showDialog(); + } catch (error) { + let message = utils.getErrorMessage(error); + await vscode.window.showErrorMessage(message); + } } } diff --git a/extensions/notebook/src/jupyter/jupyterServerInstallation.ts b/extensions/notebook/src/jupyter/jupyterServerInstallation.ts index 87b3ed7736..d3612af3af 100644 --- a/extensions/notebook/src/jupyter/jupyterServerInstallation.ts +++ b/extensions/notebook/src/jupyter/jupyterServerInstallation.ts @@ -18,6 +18,7 @@ import * as utils from '../common/utils'; import { Deferred } from '../common/promise'; import { ConfigurePythonWizard } from '../dialog/configurePython/configurePythonWizard'; import { IKernelInfo } from '../contracts/content'; +import { requiredJupyterPackages } from './requiredJupyterPackages'; const localize = nls.loadMessageBundle(); const msgInstallPkgProgress = localize('msgInstallPkgProgress', "Notebook dependencies installation is in progress"); @@ -78,30 +79,6 @@ export interface IJupyterServerInstallation { installedPythonVersion: string; } -export const requiredJupyterPkg: PythonPkgDetails = { - name: 'jupyter', - version: '1.0.0' -}; - -// Require notebook 6.5.6 for https://github.com/jupyter/notebook/issues/7048 -export const requiredNotebookPkg: PythonPkgDetails = { - name: 'notebook', - version: '6.5.6', - installExactVersion: true -}; - -// https://github.com/microsoft/azuredatastudio/issues/24405 -export const requiredIpykernelPkg: PythonPkgDetails = { - name: 'ipykernel', - version: '5.5.5', - installExactVersion: true -}; - -export const requiredPowershellPkg: PythonPkgDetails = { - name: 'powershell-kernel', - version: '0.1.4' -}; - export class JupyterServerInstallation implements IJupyterServerInstallation { public extensionPath: string; public pythonBinPath: string; @@ -121,14 +98,14 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { private _oldUserInstalledPipPackages: PythonPkgDetails[] = []; private _upgradePrompted: boolean = false; - private _installInProgress: boolean; + private _installInProgress = false; private _installCompletion: Deferred; public static readonly DefaultPythonLocation = path.join(utils.getUserHome(), 'azuredatastudio-python'); - private _kernelSetupCache: Map; - private readonly _requiredKernelPackages: Map; - private readonly _requiredPackagesSet: Set; + private readonly _kernelSetupCache = new Map(); + private readonly _requiredKernelPackages = new Map(); + private readonly _requiredPackageNames: Set; private readonly _runningOnSAW: boolean; private readonly _tsgopsweb: boolean; @@ -152,22 +129,21 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { this._pythonInstallationPath = JupyterServerInstallation.getPythonInstallPath(); this._usingExistingPython = JupyterServerInstallation.getExistingPythonSetting(); } - this._installInProgress = false; - this._kernelSetupCache = new Map(); - this._requiredKernelPackages = new Map(); + let allPackages = requiredJupyterPackages.sharedPackages; + for (let kernelInfo of requiredJupyterPackages.kernels) { + // Add this kernel's specific dependencies to the running list of packages for All Kernels. + allPackages = allPackages.concat(kernelInfo.packages); - this._requiredKernelPackages.set(constants.ipykernelDisplayName, [requiredJupyterPkg, requiredNotebookPkg, requiredIpykernelPkg]); - this._requiredKernelPackages.set(constants.python3DisplayName, [requiredJupyterPkg, requiredNotebookPkg, requiredIpykernelPkg]); - this._requiredKernelPackages.set(constants.powershellDisplayName, [requiredJupyterPkg, requiredPowershellPkg, requiredNotebookPkg, requiredIpykernelPkg]); + // Combine this kernel's specific dependencies with the shared list of packages to get its + // full list of dependencies + let packages = requiredJupyterPackages.sharedPackages.concat(kernelInfo.packages); + this._requiredKernelPackages.set(kernelInfo.name, packages); + } - let allPackages = [requiredJupyterPkg, requiredNotebookPkg, requiredIpykernelPkg, requiredPowershellPkg]; this._requiredKernelPackages.set(constants.allKernelsName, allPackages); - this._requiredPackagesSet = new Set(); - allPackages.forEach(pkg => { - this._requiredPackagesSet.add(pkg.name); - }); + this._requiredPackageNames = new Set(allPackages.map(pkg => pkg.name)); } private async installDependencies(backgroundOperation: azdata.BackgroundOperation, forceInstall: boolean, packages: PythonPkgDetails[]): Promise { @@ -461,8 +437,10 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { await notebookConfig.update(constants.existingPythonConfigKey, this._usingExistingPython, vscode.ConfigurationTarget.Global); await this.configurePackagePaths(); + await vscode.commands.executeCommand(constants.BuiltInCommands.SetContext, constants.CommandContext.NotebookPythonInstalled, true); this._installCompletion.resolve(); this._installInProgress = false; + if (this._upgradeInProcess) { // Pass in false for restartJupyterServer parameter since the jupyter server has already been shutdown // when removing the old Python version on Windows. @@ -635,7 +613,7 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { public uninstallPipPackages(packages: PythonPkgDetails[]): Promise { for (let pkg of packages) { - if (this._requiredPackagesSet.has(pkg.name)) { + if (this._requiredPackageNames.has(pkg.name)) { this._kernelSetupCache.clear(); break; } @@ -687,7 +665,7 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { public async uninstallCondaPackages(packages: PythonPkgDetails[]): Promise { if (this.condaExecutable) { for (let pkg of packages) { - if (this._requiredPackagesSet.has(pkg.name)) { + if (this._requiredPackageNames.has(pkg.name)) { this._kernelSetupCache.clear(); break; } diff --git a/extensions/notebook/src/jupyter/jupyterServerManager.ts b/extensions/notebook/src/jupyter/jupyterServerManager.ts index 2fed27947b..d135529cda 100644 --- a/extensions/notebook/src/jupyter/jupyterServerManager.ts +++ b/extensions/notebook/src/jupyter/jupyterServerManager.ts @@ -14,7 +14,6 @@ import { JupyterServerInstallation } from './jupyterServerInstallation'; import * as utils from '../common/utils'; import { IServerInstance } from './common'; import { PerFolderServerInstance, IInstanceOptions } from './serverInstance'; -import { CommandContext, BuiltInCommands } from '../common/constants'; export interface IServerManagerOptions { documentPath: string; @@ -105,7 +104,6 @@ export class LocalJupyterServerManager implements nb.ServerManager, vscode.Dispo private async doStartServer(kernelSpec: nb.IKernelSpec): Promise { // We can't find or create servers until the installation is complete let installation = this.options.jupyterInstallation; await installation.promptForPythonInstall(kernelSpec.display_name); - void vscode.commands.executeCommand(BuiltInCommands.SetContext, CommandContext.NotebookPythonInstalled, true); // Calculate the path to use as the notebook-dir for Jupyter based on the path of the uri of the // notebook to open. This will be the workspace folder if the notebook uri is inside a workspace diff --git a/extensions/notebook/src/jupyter/requiredJupyterPackages.ts b/extensions/notebook/src/jupyter/requiredJupyterPackages.ts new file mode 100644 index 0000000000..8da47e0cdf --- /dev/null +++ b/extensions/notebook/src/jupyter/requiredJupyterPackages.ts @@ -0,0 +1,46 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { PythonPkgDetails } from './jupyterServerInstallation' + +export interface RequiredPackagesInfo { + sharedPackages: PythonPkgDetails[]; + kernels: { + name: string; + packages: PythonPkgDetails[]; + }[]; +} + +export const requiredJupyterPackages: RequiredPackagesInfo = { + sharedPackages: [{ + name: 'jupyter', + version: '1.0.0' + }, + // Require notebook 6.5.6 for https://github.com/jupyter/notebook/issues/7048 + { + name: 'notebook', + version: '6.5.6', + installExactVersion: true + }, + // Require ipykernel 5.5.5 for https://github.com/microsoft/azuredatastudio/issues/24405 + { + name: 'ipykernel', + version: '5.5.5', + installExactVersion: true + }], + kernels: [{ + name: 'Python 3 (ipykernel)', + packages: [] + }, { + name: 'Python 3', + packages: [] + }, { + name: 'PowerShell', + packages: [{ + name: 'powershell-kernel', + version: '0.1.4' + }] + }] +} diff --git a/extensions/notebook/src/test/model/jupyterController.test.ts b/extensions/notebook/src/test/model/jupyterController.test.ts index caf69bf73b..60322fff3c 100644 --- a/extensions/notebook/src/test/model/jupyterController.test.ts +++ b/extensions/notebook/src/test/model/jupyterController.test.ts @@ -19,10 +19,8 @@ describe('Jupyter Controller', function () { let appContext = new AppContext(mockExtensionContext); let controller: JupyterController; let connection: azdata.connection.ConnectionProfile = new azdata.connection.ConnectionProfile(); - let showErrorMessageSpy: sinon.SinonSpy; this.beforeEach(() => { - showErrorMessageSpy = sinon.spy(vscode.window, 'showErrorMessage'); sinon.stub(azdata.connection, 'getCurrentConnection').returns(Promise.resolve(connection)); sinon.stub(azdata.tasks, 'registerTask'); sinon.stub(vscode.commands, 'registerCommand'); @@ -62,17 +60,6 @@ describe('Jupyter Controller', function () { should(defaultConnection).deepEqual(connection, 'getDefaultConnection() did not return expected result'); }); - it('should show error message for doManagePackages before activation', async () => { - await controller.doManagePackages(); - should(showErrorMessageSpy.calledOnce).be.true('showErrorMessage should be called'); - }); - - it('should not show error message for doManagePackages after activation', async () => { - await controller.activate(); - await controller.doManagePackages(); - should(showErrorMessageSpy.notCalled).be.true('showErrorMessage should not be called'); - }); - it('Returns expected values from notebook provider', async () => { await controller.activate(); should(controller.executeProvider.providerId).equal('jupyter', 'Notebook provider should be jupyter'); diff --git a/extensions/notebook/src/test/python/jupyterInstallation.test.ts b/extensions/notebook/src/test/python/jupyterInstallation.test.ts index 8d5f8d339c..21897ea6d3 100644 --- a/extensions/notebook/src/test/python/jupyterInstallation.test.ts +++ b/extensions/notebook/src/test/python/jupyterInstallation.test.ts @@ -11,8 +11,9 @@ import * as uuid from 'uuid'; import * as fs from 'fs-extra'; import * as request from 'request'; import * as utils from '../../common/utils'; -import { requiredJupyterPkg, JupyterServerInstallation, requiredPowershellPkg, PythonInstallSettings, PythonPkgDetails, requiredNotebookPkg, requiredIpykernelPkg } from '../../jupyter/jupyterServerInstallation'; -import { powershellDisplayName, python3DisplayName, winPlatform } from '../../common/constants'; +import { JupyterServerInstallation, PythonInstallSettings, PythonPkgDetails } from '../../jupyter/jupyterServerInstallation'; +import { allKernelsName, ipykernelDisplayName, powershellDisplayName, python3DisplayName, winPlatform } from '../../common/constants'; +import { requiredJupyterPackages } from '../../jupyter/requiredJupyterPackages'; describe('Jupyter Server Installation', function () { let outputChannelStub: TypeMoq.IMock; @@ -224,14 +225,24 @@ describe('Jupyter Server Installation', function () { should(packages.length).be.equal(0); }); - it('Get required packages test - Python 3 kernel', async function () { - let packages = installation.getRequiredPackagesForKernel(python3DisplayName); - should(packages).be.deepEqual([requiredJupyterPkg, requiredNotebookPkg, requiredIpykernelPkg]); + const pythonKernels = [python3DisplayName, ipykernelDisplayName, powershellDisplayName]; + pythonKernels.forEach(kernelName => { + it(`Get required packages test - ${kernelName} kernel`, async function () { + let packages = installation.getRequiredPackagesForKernel(kernelName); + let kernelInfo = requiredJupyterPackages.kernels.find(kernel => kernel.name === kernelName); + should(kernelInfo).not.be.undefined(); + let expectedPackages = requiredJupyterPackages.sharedPackages.concat(kernelInfo.packages); + should(packages).be.deepEqual(expectedPackages); + }); }); - it('Get required packages test - Powershell kernel', async function () { - let packages = installation.getRequiredPackagesForKernel(powershellDisplayName); - should(packages).be.deepEqual([requiredJupyterPkg, requiredPowershellPkg, requiredNotebookPkg, requiredIpykernelPkg]); + it('Get required packages test - All Kernels', async function () { + let packages = installation.getRequiredPackagesForKernel(allKernelsName); + let allPackages = requiredJupyterPackages.sharedPackages; + for (let kernel of requiredJupyterPackages.kernels) { + allPackages = allPackages.concat(kernel.packages); + } + should(packages).be.deepEqual(allPackages); }); it('Install python test - Run install while Python is already running', async function () {