From 4189e761ff01447ff018c494e50ebb980523b301 Mon Sep 17 00:00:00 2001 From: Cory Rivera Date: Wed, 3 Jul 2019 12:51:48 -0700 Subject: [PATCH] Fix bugs in selecting a system version of Python for Notebook dependencies (#6250) --- .../notebook/src/jupyter/jupyterController.ts | 1 + .../src/jupyter/jupyterServerInstallation.ts | 111 +++++++++++------- .../src/jupyter/jupyterServerManager.ts | 13 +- 3 files changed, 83 insertions(+), 42 deletions(-) diff --git a/extensions/notebook/src/jupyter/jupyterController.ts b/extensions/notebook/src/jupyter/jupyterController.ts index 9bbf3b8363..87809467b3 100644 --- a/extensions/notebook/src/jupyter/jupyterController.ts +++ b/extensions/notebook/src/jupyter/jupyterController.ts @@ -58,6 +58,7 @@ export class JupyterController implements vscode.Disposable { this.extensionContext.extensionPath, this.outputChannel, this.apiWrapper); + await this._jupyterInstallation.configurePackagePaths(); // Add command/task handlers this.apiWrapper.registerTaskHandler(constants.jupyterOpenNotebookTask, (profile: azdata.IConnectionProfile) => { diff --git a/extensions/notebook/src/jupyter/jupyterServerInstallation.ts b/extensions/notebook/src/jupyter/jupyterServerInstallation.ts index 4a78733991..a9346a9e60 100644 --- a/extensions/notebook/src/jupyter/jupyterServerInstallation.ts +++ b/extensions/notebook/src/jupyter/jupyterServerInstallation.ts @@ -64,8 +64,6 @@ export class JupyterServerInstallation { this._usingConda = false; this._installInProgress = false; this._usingExistingPython = JupyterServerInstallation.getExistingPythonSetting(this.apiWrapper); - - this.configurePackagePaths(); } private async installDependencies(backgroundOperation: azdata.BackgroundOperation): Promise { @@ -78,22 +76,23 @@ export class JupyterServerInstallation { try { await this.installPythonPackage(backgroundOperation); + + this.outputChannel.appendLine(msgPythonDownloadComplete); + backgroundOperation.updateStatus(azdata.TaskStatus.InProgress, msgPythonDownloadComplete); + + if (this._usingConda) { + await this.installCondaDependencies(); + } else if (this._usingExistingPython) { + await this.installPipDependencies(); + } else { + await this.installOfflinePipDependencies(); + } + let doOnlineInstall = this._usingExistingPython; + await this.installSparkMagic(doOnlineInstall); } catch (err) { this.outputChannel.appendLine(msgDependenciesInstallationFailed(utils.getErrorMessage(err))); throw err; } - this.outputChannel.appendLine(msgPythonDownloadComplete); - backgroundOperation.updateStatus(azdata.TaskStatus.InProgress, msgPythonDownloadComplete); - - if (this._usingConda) { - await this.installCondaDependencies(); - } else if (this._usingExistingPython) { - await this.installPipDependencies(); - } else { - await this.installOfflinePipDependencies(); - } - let doOnlineInstall = this._usingExistingPython; - await this.installSparkMagic(doOnlineInstall); fs.remove(this._pythonPackageDir, (err: Error) => { if (err) { @@ -134,10 +133,17 @@ export class JupyterServerInstallation { } } - let pythonPackagePathLocal = this._pythonInstallationPath + '/' + packageName; + let installPath: string; + if (this._usingExistingPython) { + installPath = utils.getUserHome(); + } else { + installPath = this._pythonInstallationPath; + } + + let pythonPackagePathLocal = path.join(installPath, packageName); return new Promise((resolve, reject) => { backgroundOperation.updateStatus(azdata.TaskStatus.InProgress, msgDownloadPython(platformId, pythonDownloadUrl)); - fs.mkdirs(this._pythonInstallationPath, (err) => { + fs.mkdirs(installPath, (err) => { if (err) { backgroundOperation.updateStatus(azdata.TaskStatus.InProgress, msgPythonDirectoryError); return reject(err); @@ -177,7 +183,7 @@ export class JupyterServerInstallation { .on('close', () => { //unpack python zip/tar file this.outputChannel.appendLine(msgPythonUnpackPending); - let pythonSourcePath = path.join(this._pythonInstallationPath, constants.pythonBundleVersion); + let pythonSourcePath = path.join(installPath, constants.pythonBundleVersion); if (!this._usingExistingPython && fs.existsSync(pythonSourcePath)) { try { fs.removeSync(pythonSourcePath); @@ -186,7 +192,7 @@ export class JupyterServerInstallation { return reject(err); } } - decompress(pythonPackagePathLocal, this._pythonInstallationPath).then(files => { + decompress(pythonPackagePathLocal, installPath).then(files => { //Delete zip/tar file fs.unlink(pythonPackagePathLocal, (err) => { if (err) { @@ -210,13 +216,17 @@ export class JupyterServerInstallation { }); } - private configurePackagePaths(): void { + public async configurePackagePaths(): Promise { //Python source path up to bundle version let pythonSourcePath = this._usingExistingPython ? this._pythonInstallationPath : path.join(this._pythonInstallationPath, constants.pythonBundleVersion); - this._pythonPackageDir = path.join(pythonSourcePath, 'offlinePackages'); + if (this._usingExistingPython) { + this._pythonPackageDir = path.join(utils.getUserHome(), 'offlinePackages'); + } else { + this._pythonPackageDir = path.join(pythonSourcePath, 'offlinePackages'); + } // Update python paths and properties to reference user's local python. let pythonBinPathSuffix = process.platform === constants.winPlatform ? '' : 'bin'; @@ -246,6 +256,13 @@ export class JupyterServerInstallation { } } + if (this._usingExistingPython) { + let pythonUserDir = await this.getPythonUserDir(this._pythonExecutable); + if (pythonUserDir) { + this.pythonEnvVarPath = pythonUserDir + delimiter + this.pythonEnvVarPath; + } + } + // Delete existing Python variables in ADS to prevent conflict with other installs delete process.env['PYTHONPATH']; delete process.env['PYTHONSTARTUP']; @@ -261,19 +278,7 @@ export class JupyterServerInstallation { } private isPythonRunning(pythonInstallPath: string): Promise { - let pythonExePath = JupyterServerInstallation.getPythonExePath(pythonInstallPath, this._usingExistingPython); - return new Promise(resolve => { - fs.open(pythonExePath, 'r+', (err, fd) => { - if (!err) { - fs.close(fd, err => { - this.apiWrapper.showErrorMessage(utils.getErrorMessage(err)); - }); - resolve(false); - } else { - resolve(err.code === 'EBUSY' || err.code === 'EPERM'); - } - }); - }); + return Promise.resolve(false); } /** @@ -298,7 +303,7 @@ export class JupyterServerInstallation { this._pythonInstallationPath = installSettings.installPath; this._usingExistingPython = installSettings.existingPython; } - this.configurePackagePaths(); + await this.configurePackagePaths(); let updateConfig = async () => { let notebookConfig = this.apiWrapper.getConfiguration(constants.notebookConfigKey); @@ -359,7 +364,7 @@ export class JupyterServerInstallation { } public installPipPackage(packageName: string, version: string): Promise { - let cmd = `"${this.pythonExecutable}" -m pip install ${packageName}==${version}`; + let cmd = `"${this.pythonExecutable}" -m pip install --user ${packageName}==${version}`; return this.executeStreamedCommand(cmd); } @@ -402,7 +407,7 @@ export class JupyterServerInstallation { let installJupyterCommand: string; if (process.platform === constants.winPlatform) { let requirements = path.join(this._pythonPackageDir, 'requirements.txt'); - installJupyterCommand = `"${this._pythonExecutable}" -m pip install --no-index -r "${requirements}" --find-links "${this._pythonPackageDir}" --no-warn-script-location`; + installJupyterCommand = `"${this._pythonExecutable}" -m pip install --user --no-index -r "${requirements}" --find-links "${this._pythonPackageDir}" --no-warn-script-location`; } if (installJupyterCommand) { @@ -420,9 +425,9 @@ export class JupyterServerInstallation { if (process.platform === constants.winPlatform || this._usingExistingPython) { let sparkWheel = path.join(this._pythonPackageDir, `sparkmagic-${constants.sparkMagicVersion}-py3-none-any.whl`); if (doOnlineInstall) { - installSparkMagic = `"${this._pythonExecutable}" -m pip install "${sparkWheel}" --no-warn-script-location`; + installSparkMagic = `"${this._pythonExecutable}" -m pip install --user "${sparkWheel}" --no-warn-script-location`; } else { - installSparkMagic = `"${this._pythonExecutable}" -m pip install --no-index "${sparkWheel}" --find-links "${this._pythonPackageDir}" --no-warn-script-location`; + installSparkMagic = `"${this._pythonExecutable}" -m pip install --user --no-index "${sparkWheel}" --find-links "${this._pythonPackageDir}" --no-warn-script-location`; } } @@ -437,10 +442,10 @@ export class JupyterServerInstallation { this.outputChannel.show(true); this.outputChannel.appendLine(localize('msgInstallStart', "Installing required packages to run Notebooks...")); - let installCommand = `"${this._pythonExecutable}" -m pip install jupyter==1.0.0 pandas==0.24.2`; + let installCommand = `"${this._pythonExecutable}" -m pip install --user jupyter==1.0.0 pandas==0.24.2`; await this.executeStreamedCommand(installCommand); - installCommand = `"${this._pythonExecutable}" -m pip install prose-codeaccelerator==1.3.0 --extra-index-url https://prose-python-packages.azurewebsites.net`; + installCommand = `"${this._pythonExecutable}" -m pip install --user prose-codeaccelerator==1.3.0 --extra-index-url https://prose-python-packages.azurewebsites.net`; await this.executeStreamedCommand(installCommand); this.outputChannel.appendLine(localize('msgJupyterInstallDone', "... Jupyter installation complete.")); @@ -456,7 +461,7 @@ export class JupyterServerInstallation { } await this.executeStreamedCommand(installCommand); - installCommand = `"${this._pythonExecutable}" -m pip install prose-codeaccelerator==1.3.0 --extra-index-url https://prose-python-packages.azurewebsites.net`; + installCommand = `"${this._pythonExecutable}" -m pip install --user prose-codeaccelerator==1.3.0 --extra-index-url https://prose-python-packages.azurewebsites.net`; await this.executeStreamedCommand(installCommand); this.outputChannel.appendLine(localize('msgJupyterInstallDone', "... Jupyter installation complete.")); @@ -565,6 +570,30 @@ export class JupyterServerInstallation { useExistingInstall ? '' : constants.pythonBundleVersion, process.platform === constants.winPlatform ? 'python.exe' : 'bin/python3'); } + + private async getPythonUserDir(pythonExecutable: string): Promise { + let sitePath: string; + if (process.platform === constants.winPlatform) { + sitePath = 'USER_SITE'; + } else { + sitePath = 'USER_BASE'; + } + let cmd = `"${pythonExecutable}" -c "import site;print(site.${sitePath})"`; + + let packagesDir = await utils.executeBufferedCommand(cmd, {}); + if (packagesDir && packagesDir.length > 0) { + packagesDir = packagesDir.trim(); + if (process.platform === constants.winPlatform) { + packagesDir = path.resolve(path.join(packagesDir, '..', 'Scripts')); + } else { + packagesDir = path.join(packagesDir, 'bin'); + } + + return packagesDir; + } + + return undefined; + } } export interface PythonPkgDetails { diff --git a/extensions/notebook/src/jupyter/jupyterServerManager.ts b/extensions/notebook/src/jupyter/jupyterServerManager.ts index 1084f0e8a5..09f5b04473 100644 --- a/extensions/notebook/src/jupyter/jupyterServerManager.ts +++ b/extensions/notebook/src/jupyter/jupyterServerManager.ts @@ -118,7 +118,18 @@ export class LocalJupyterServerManager implements nb.ServerManager, vscode.Dispo // /path2/nb3.ipynb // ... will result in 2 notebook servers being started, one for /path1/ and one for /path2/ let notebookDir = this.apiWrapper.getWorkspacePathFromUri(vscode.Uri.file(this.documentPath)); - notebookDir = notebookDir || path.dirname(this.documentPath); + if (!notebookDir) { + let docDir = path.dirname(this.documentPath); + if (docDir === '.') { + // If the user is using a system version of python, then + // '.' will try to create a notebook in a system directory. + // Since this will fail due to permissions, use the user's + // home folder instead. + notebookDir = utils.getUserHome(); + } else { + notebookDir = docDir; + } + } // TODO handle notification of start/stop status // notebookContext.updateLoadingMessage(localizedConstants.msgJupyterStarting);