From f409abfc01f6d4c46fad258b9ecdc001ce788a98 Mon Sep 17 00:00:00 2001 From: Cory Rivera Date: Tue, 7 Jul 2020 11:32:05 -0700 Subject: [PATCH] Disable python downloads when running notebooks on a SAW. (#11186) --- extensions/notebook/package.json | 10 +++- .../resources/pythonScripts/startNotebook.py | 2 + extensions/notebook/src/common/apiWrapper.ts | 5 -- .../src/jupyter/jupyterServerInstallation.ts | 55 ++++++++++--------- .../src/jupyter/jupyterServerManager.ts | 4 +- .../notebook/src/jupyter/serverInstance.ts | 7 ++- .../managePackagesDialogModel.test.ts | 2 +- .../src/test/model/serverInstance.test.ts | 2 +- 8 files changed, 48 insertions(+), 39 deletions(-) create mode 100644 extensions/notebook/resources/pythonScripts/startNotebook.py diff --git a/extensions/notebook/package.json b/extensions/notebook/package.json index 08429a2344..9bc8b3ee70 100644 --- a/extensions/notebook/package.json +++ b/extensions/notebook/package.json @@ -302,6 +302,14 @@ "command": "jupyter.task.openNotebook", "when": "false" }, + { + "command": "jupyter.cmd.configurePython", + "when": "!notebook:runningOnSAW" + }, + { + "command": "jupyter.reinstallDependencies", + "when": "!notebook:runningOnSAW" + }, { "command": "jupyter.cmd.managePackages", "when": "false" @@ -418,7 +426,7 @@ "notebook/toolbar": [ { "command": "jupyter.cmd.managePackages", - "when": "providerId == jupyter && notebook:pythonInstalled" + "when": "providerId == jupyter && notebook:pythonInstalled && !notebook:runningOnSAW" } ] }, diff --git a/extensions/notebook/resources/pythonScripts/startNotebook.py b/extensions/notebook/resources/pythonScripts/startNotebook.py new file mode 100644 index 0000000000..a76d3a34de --- /dev/null +++ b/extensions/notebook/resources/pythonScripts/startNotebook.py @@ -0,0 +1,2 @@ +import notebook.notebookapp +notebook.notebookapp.main() diff --git a/extensions/notebook/src/common/apiWrapper.ts b/extensions/notebook/src/common/apiWrapper.ts index 392a661189..147dbb06b8 100644 --- a/extensions/notebook/src/common/apiWrapper.ts +++ b/extensions/notebook/src/common/apiWrapper.ts @@ -5,7 +5,6 @@ import * as vscode from 'vscode'; import * as azdata from 'azdata'; -import { CommandContext, BuiltInCommands } from './constants'; /** * Wrapper class to act as a facade over VSCode and Data APIs and allow us to test / mock callbacks into @@ -61,10 +60,6 @@ export class ApiWrapper { azdata.tasks.startBackgroundOperation(operationInfo); } - public setCommandContext(key: CommandContext | string, value: any): Thenable { - return vscode.commands.executeCommand(BuiltInCommands.SetContext, key, value); - } - public getNotebookDocuments() { return azdata.nb.notebookDocuments; } diff --git a/extensions/notebook/src/jupyter/jupyterServerInstallation.ts b/extensions/notebook/src/jupyter/jupyterServerInstallation.ts index 1c6f06a83d..58aeac8e1c 100644 --- a/extensions/notebook/src/jupyter/jupyterServerInstallation.ts +++ b/extensions/notebook/src/jupyter/jupyterServerInstallation.ts @@ -3,6 +3,7 @@ * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import * as vscode from 'vscode'; import * as fs from 'fs-extra'; import * as path from 'path'; import * as nls from 'vscode-nls'; @@ -15,7 +16,6 @@ import * as tar from 'tar'; import { ApiWrapper } from '../common/apiWrapper'; import * as constants from '../common/constants'; import * as utils from '../common/utils'; -import { OutputChannel, ConfigurationTarget, window } from 'vscode'; import { Deferred } from '../common/promise'; import { ConfigurePythonWizard } from '../dialog/configurePython/configurePythonWizard'; import { IPrompter, IQuestion, confirm } from '../prompts/question'; @@ -59,13 +59,13 @@ export interface IJupyterServerInstallation { uninstallPipPackages(packages: PythonPkgDetails[]): Promise; pythonExecutable: string; pythonInstallationPath: string; - installPythonPackage(backgroundOperation: azdata.BackgroundOperation, usingExistingPython: boolean, pythonInstallationPath: string, outputChannel: OutputChannel): Promise; + installPythonPackage(backgroundOperation: azdata.BackgroundOperation, usingExistingPython: boolean, pythonInstallationPath: string, outputChannel: vscode.OutputChannel): Promise; } export class JupyterServerInstallation implements IJupyterServerInstallation { public apiWrapper: ApiWrapper; public extensionPath: string; public pythonBinPath: string; - public outputChannel: OutputChannel; + public outputChannel: vscode.OutputChannel; public pythonEnvVarPath: string; public execOptions: ExecOptions; @@ -112,14 +112,25 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { private readonly _requiredKernelPackages: Map; private readonly _requiredPackagesSet: Set; - constructor(extensionPath: string, outputChannel: OutputChannel, apiWrapper: ApiWrapper, pythonInstallationPath?: string) { + private readonly _runningOnSAW: boolean; + + constructor(extensionPath: string, outputChannel: vscode.OutputChannel, apiWrapper: ApiWrapper) { this.extensionPath = extensionPath; this.outputChannel = outputChannel; this.apiWrapper = apiWrapper; - this._pythonInstallationPath = pythonInstallationPath || JupyterServerInstallation.getPythonInstallPath(this.apiWrapper); + + this._runningOnSAW = vscode.env.appName.toLowerCase().indexOf('saw') > 0; + vscode.commands.executeCommand(constants.BuiltInCommands.SetContext, 'notebook:runningOnSAW', this._runningOnSAW); + + if (this._runningOnSAW) { + this._pythonInstallationPath = `${vscode.env.appRoot}\\ads-python`; + this._usingExistingPython = true; + } else { + this._pythonInstallationPath = JupyterServerInstallation.getPythonInstallPath(this.apiWrapper); + this._usingExistingPython = JupyterServerInstallation.getExistingPythonSetting(this.apiWrapper); + } this._usingConda = false; this._installInProgress = false; - this._usingExistingPython = JupyterServerInstallation.getExistingPythonSetting(this.apiWrapper); this._prompter = new CodeAdapter(); @@ -170,7 +181,7 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { } private async installDependencies(backgroundOperation: azdata.BackgroundOperation, forceInstall: boolean, specificPackages?: PythonPkgDetails[]): Promise { - window.showInformationMessage(msgInstallPkgStart); + vscode.window.showInformationMessage(msgInstallPkgStart); this.outputChannel.show(true); this.outputChannel.appendLine(msgInstallPkgProgress); @@ -189,10 +200,10 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { this.outputChannel.appendLine(msgInstallPkgFinish); backgroundOperation.updateStatus(azdata.TaskStatus.Succeeded, msgInstallPkgFinish); - window.showInformationMessage(msgInstallPkgFinish); + vscode.window.showInformationMessage(msgInstallPkgFinish); } - public installPythonPackage(backgroundOperation: azdata.BackgroundOperation, usingExistingPython: boolean, pythonInstallationPath: string, outputChannel: OutputChannel): Promise { + public installPythonPackage(backgroundOperation: azdata.BackgroundOperation, usingExistingPython: boolean, pythonInstallationPath: string, outputChannel: vscode.OutputChannel): Promise { if (usingExistingPython) { return Promise.resolve(); } @@ -435,8 +446,8 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { this.installDependencies(op, forceInstall, installSettings.specificPackages) .then(async () => { let notebookConfig = this.apiWrapper.getConfiguration(constants.notebookConfigKey); - await notebookConfig.update(constants.pythonPathConfigKey, this._pythonInstallationPath, ConfigurationTarget.Global); - await notebookConfig.update(constants.existingPythonConfigKey, this._usingExistingPython, ConfigurationTarget.Global); + await notebookConfig.update(constants.pythonPathConfigKey, this._pythonInstallationPath, vscode.ConfigurationTarget.Global); + await notebookConfig.update(constants.existingPythonConfigKey, this._usingExistingPython, vscode.ConfigurationTarget.Global); await this.configurePackagePaths(); this._installCompletion.resolve(); @@ -457,6 +468,9 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { * Opens a dialog for configuring the installation path for the Notebook Python dependencies. */ public async promptForPythonInstall(kernelDisplayName: string): Promise { + if (this._runningOnSAW) { + return Promise.resolve(); + } if (this._installInProgress) { this.apiWrapper.showInfoMessage(msgWaitingForInstall); return this._installCompletion.promise; @@ -482,6 +496,9 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { * Prompts user to upgrade certain python packages if they're below the minimum expected version. */ public async promptForPackageUpgrade(kernelName: string): Promise { + if (this._runningOnSAW) { + return Promise.resolve(); + } if (this._installInProgress) { this.apiWrapper.showInfoMessage(msgWaitingForInstall); return this._installCompletion.promise; @@ -835,22 +852,6 @@ export class JupyterServerInstallation implements IJupyterServerInstallation { return path; } - /** - * Returns the folder containing the python executable under the path defined in - * "notebook.pythonPath" in the user's settings. - * @param apiWrapper An ApiWrapper to use when retrieving user settings info. - */ - public static getPythonBinPath(apiWrapper: ApiWrapper): string { - let pythonBinPathSuffix = process.platform === constants.winPlatform ? '' : 'bin'; - - let useExistingInstall = JupyterServerInstallation.getExistingPythonSetting(apiWrapper); - - return path.join( - JupyterServerInstallation.getPythonInstallPath(apiWrapper), - useExistingInstall ? '' : constants.pythonBundleVersion, - pythonBinPathSuffix); - } - public static getPythonExePath(pythonInstallPath: string, useExistingInstall: boolean): string { return path.join( pythonInstallPath, diff --git a/extensions/notebook/src/jupyter/jupyterServerManager.ts b/extensions/notebook/src/jupyter/jupyterServerManager.ts index 83aed59f47..c45241316b 100644 --- a/extensions/notebook/src/jupyter/jupyterServerManager.ts +++ b/extensions/notebook/src/jupyter/jupyterServerManager.ts @@ -15,7 +15,7 @@ import { JupyterServerInstallation } from './jupyterServerInstallation'; import * as utils from '../common/utils'; import { IServerInstance } from './common'; import { PerFolderServerInstance, IInstanceOptions } from './serverInstance'; -import { CommandContext } from '../common/constants'; +import { CommandContext, BuiltInCommands } from '../common/constants'; export interface IServerManagerOptions { documentPath: string; @@ -112,7 +112,7 @@ export class LocalJupyterServerManager implements nb.ServerManager, vscode.Dispo if (!installation.previewFeaturesEnabled) { await installation.promptForPackageUpgrade(kernelSpec.display_name); } - this._apiWrapper.setCommandContext(CommandContext.NotebookPythonInstalled, true); + 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/serverInstance.ts b/extensions/notebook/src/jupyter/serverInstance.ts index 86a435dae8..f278c479af 100644 --- a/extensions/notebook/src/jupyter/serverInstance.ts +++ b/extensions/notebook/src/jupyter/serverInstance.ts @@ -127,8 +127,11 @@ export class PerFolderServerInstance implements IServerInstance { private childProcess: ChildProcess; private errorHandler: ErrorHandler = new ErrorHandler(); + private readonly notebookScriptPath: string; + constructor(private options: IInstanceOptions, fsUtils?: ServerInstanceUtils) { this.utils = fsUtils || new ServerInstanceUtils(); + this.notebookScriptPath = path.join(this.options.install.extensionPath, 'resources', 'pythonScripts', 'startNotebook.py'); } public get isStarted(): boolean { @@ -162,7 +165,7 @@ export class PerFolderServerInstance implements IServerInstance { } if (this._isStarted) { let install = this.options.install; - let stopCommand = `"${install.pythonExecutable}" -m jupyter notebook stop ${this._port}`; + let stopCommand = `"${install.pythonExecutable}" "${this.notebookScriptPath}" stop ${this._port}`; await this.utils.executeBufferedCommand(stopCommand, install.execOptions, install.outputChannel); } } catch (error) { @@ -243,7 +246,7 @@ export class PerFolderServerInstance implements IServerInstance { let token = await utils.getRandomToken(); this._uri = vscode.Uri.parse(`http://localhost:${port}/?token=${token}`); this._port = port.toString(); - let startCommand = `"${this.options.install.pythonExecutable}" -m jupyter notebook --no-browser --no-mathjax --notebook-dir "${notebookDirectory}" --port=${port} --NotebookApp.token=${token}`; + let startCommand = `"${this.options.install.pythonExecutable}" "${this.notebookScriptPath}" --no-browser --no-mathjax --notebook-dir "${notebookDirectory}" --port=${port} --NotebookApp.token=${token}`; this.notifyStarting(this.options.install, startCommand); // Execute the command diff --git a/extensions/notebook/src/test/managePackages/managePackagesDialogModel.test.ts b/extensions/notebook/src/test/managePackages/managePackagesDialogModel.test.ts index 7ea7f6a894..f5cd8f7c25 100644 --- a/extensions/notebook/src/test/managePackages/managePackagesDialogModel.test.ts +++ b/extensions/notebook/src/test/managePackages/managePackagesDialogModel.test.ts @@ -19,7 +19,7 @@ interface TestContext { describe('Manage Packages', () => { let jupyterServerInstallation: JupyterServerInstallation; beforeEach(() => { - jupyterServerInstallation = new JupyterServerInstallation(undefined, undefined, undefined, undefined); + jupyterServerInstallation = new JupyterServerInstallation(undefined, undefined, undefined); }); it('Should throw exception given undefined providers', async function (): Promise { diff --git a/extensions/notebook/src/test/model/serverInstance.test.ts b/extensions/notebook/src/test/model/serverInstance.test.ts index 05c4a9f926..99f4596892 100644 --- a/extensions/notebook/src/test/model/serverInstance.test.ts +++ b/extensions/notebook/src/test/model/serverInstance.test.ts @@ -147,7 +147,7 @@ describe('Jupyter server instance', function (): void { await serverInstance.stop(); // Then I expect stop to be called on the child process - should(actualCommand.indexOf(`jupyter notebook stop ${serverInstance.port}`)).be.greaterThan(-1); + should(actualCommand.includes(`stop ${serverInstance.port}`)).be.true('Command did not contain specified port.'); mockUtils.verify(u => u.removeDir(TypeMoq.It.isAny()), TypeMoq.Times.never()); });