Prompt for Python installation after choosing a Jupyter kernel in notebook (#4453)

extensions/notebook/src/dialog/configurePythonDialog.ts

@@ -11,8 +11,9 @@ import * as azdata from 'azdata';
 import * as fs from 'fs';
 import * as utils from '../common/utils';
 
-import { AppContext } from '../common/appContext';
 import JupyterServerInstallation from '../jupyter/jupyterServerInstallation';
+import { ApiWrapper } from '../common/apiWrapper';
+import { Deferred } from '../common/promise';
 
 const localize = nls.loadMessageBundle();
 
@@ -31,27 +32,44 @@ export class ConfigurePythonDialog {
 	private pythonLocationTextBox: azdata.InputBoxComponent;
 	private browseButton: azdata.ButtonComponent;
 
-	constructor(private appContext: AppContext, private outputChannel: vscode.OutputChannel, private jupyterInstallation: JupyterServerInstallation) {
+	private _setupComplete: Deferred<void>;
+
+	constructor(private apiWrapper: ApiWrapper, private outputChannel: vscode.OutputChannel, private jupyterInstallation: JupyterServerInstallation) {
+		this._setupComplete = new Deferred<void>();
 	}
 
-	public async showDialog() {
+	/**
+	 * Opens a dialog to configure python installation for notebooks.
+	 * @param rejectOnCancel Specifies whether an error should be thrown after clicking Cancel.
+	 * @returns A promise that is resolved when the python installation completes.
+	 */
+	public showDialog(rejectOnCancel: boolean = false): Promise<void> {
 		this.dialog = azdata.window.createModelViewDialog(this.DialogTitle);
 
 		this.initializeContent();
 
 		this.dialog.okButton.label = this.OkButtonText;
 		this.dialog.cancelButton.label = this.CancelButtonText;
+		this.dialog.cancelButton.onClick(() => {
+			if (rejectOnCancel) {
+				this._setupComplete.reject(localize('pythonInstallDeclined', 'Python installation was declined.'));
+			} else {
+				this._setupComplete.resolve();
+			}
+		});
 
 		this.dialog.registerCloseValidator(() => this.handleInstall());
 
 		azdata.window.openDialog(this.dialog);
+
+		return this._setupComplete.promise;
 	}
 
-	private initializeContent() {
+	private initializeContent(): void {
 		this.dialog.registerContent(async view => {
 			this.pythonLocationTextBox = view.modelBuilder.inputBox()
 				.withProperties<azdata.InputBoxProperties>({
-					value: JupyterServerInstallation.getPythonInstallPath(this.appContext.apiWrapper),
+					value: JupyterServerInstallation.getPythonInstallPath(this.apiWrapper),
 					width: '100%'
 				}).component();
 
@@ -106,14 +124,18 @@ export class ConfigurePythonDialog {
 				return false;
 			}
 		} catch (err) {
-			this.appContext.apiWrapper.showErrorMessage(utils.getErrorMessage(err));
+			this.apiWrapper.showErrorMessage(utils.getErrorMessage(err));
 			return false;
 		}
 
 		// Don't wait on installation, since there's currently no Cancel functionality
-		this.jupyterInstallation.startInstallProcess(pythonLocation).catch(err => {
-			this.appContext.apiWrapper.showErrorMessage(utils.getErrorMessage(err));
-		});
+		this.jupyterInstallation.startInstallProcess(pythonLocation)
+			.then(() => {
+				this._setupComplete.resolve();
+			})
+			.catch(err => {
+				this._setupComplete.reject(utils.getErrorMessage(err));
+			});
 		return true;
 	}
 
@@ -149,20 +171,13 @@ export class ConfigurePythonDialog {
 			openLabel: this.SelectFileLabel
 		};
 
-		let fileUris: vscode.Uri[] = await this.appContext.apiWrapper.showOpenDialog(options);
+		let fileUris: vscode.Uri[] = await this.apiWrapper.showOpenDialog(options);
 		if (fileUris && fileUris[0]) {
 			this.pythonLocationTextBox.value = fileUris[0].fsPath;
 		}
 	}
 
-	private showInfoMessage(message: string) {
-		this.dialog.message = {
-			text: message,
-			level: azdata.window.MessageLevel.Information
-		};
-	}
-
-	private showErrorMessage(message: string) {
+	private showErrorMessage(message: string): void {
 		this.dialog.message = {
 			text: message,
 			level: azdata.window.MessageLevel.Error

extensions/notebook/src/integrationTest/notebookIntegration.test.ts

@@ -127,6 +127,6 @@ function writeNotebookToFile(pythonNotebook: INotebook): vscode.Uri {
 async function ensureJupyterInstalled(): Promise<void> {
 	let jupterControllerExports = vscode.extensions.getExtension('Microsoft.sql-vnext').exports;
 	let jupyterController = jupterControllerExports.getJupterController() as JupyterController;
-	await jupyterController.jupyterInstallation;
+	await jupyterController.jupyterInstallation.installReady;
 }
 

extensions/notebook/src/jupyter/jupyterController.ts

@@ -30,7 +30,7 @@ import CodeAdapter from '../prompts/adapter';
 let untitledCounter = 0;
 
 export class JupyterController implements vscode.Disposable {
-	private _jupyterInstallation: Promise<JupyterServerInstallation>;
+	private _jupyterInstallation: JupyterServerInstallation;
 	private _notebookInstances: IServerInstance[] = [];
 
 	private outputChannel: vscode.OutputChannel;
@@ -55,31 +55,11 @@ export class JupyterController implements vscode.Disposable {
 
 	// PUBLIC METHODS //////////////////////////////////////////////////////
 	public async activate(): Promise<boolean> {
-		// Prompt for install if the python installation path is not defined
-		let jupyterInstaller = new JupyterServerInstallation(
+		this._jupyterInstallation = new JupyterServerInstallation(
 			this.extensionContext.extensionPath,
 			this.outputChannel,
 			this.apiWrapper);
-		if (JupyterServerInstallation.isPythonInstalled(this.apiWrapper)) {
-			this._jupyterInstallation = Promise.resolve(jupyterInstaller);
-		} else {
-			this._jupyterInstallation = new Promise(resolve => {
-				jupyterInstaller.onInstallComplete(err => {
-					if (!err) {
-						resolve(jupyterInstaller);
-					}
-				});
-			});
-		}
 
-		let notebookProvider = undefined;
-
-		notebookProvider = this.registerNotebookProvider();
-		azdata.nb.onDidOpenNotebookDocument(notebook => {
-			if (!JupyterServerInstallation.isPythonInstalled(this.apiWrapper)) {
-				this.doConfigurePython(jupyterInstaller);
-			}
-		});
 		// Add command/task handlers
 		this.apiWrapper.registerTaskHandler(constants.jupyterOpenNotebookTask, (profile: azdata.IConnectionProfile) => {
 			return this.handleOpenNotebookTask(profile);
@@ -96,11 +76,12 @@ export class JupyterController implements vscode.Disposable {
 
 		this.apiWrapper.registerCommand(constants.jupyterReinstallDependenciesCommand, () => { return this.handleDependenciesReinstallation(); });
 		this.apiWrapper.registerCommand(constants.jupyterInstallPackages, () => { return this.doManagePackages(); });
-		this.apiWrapper.registerCommand(constants.jupyterConfigurePython, () => { return this.doConfigurePython(jupyterInstaller); });
+		this.apiWrapper.registerCommand(constants.jupyterConfigurePython, () => { return this.doConfigurePython(this._jupyterInstallation); });
 
 		let supportedFileFilter: vscode.DocumentFilter[] = [
 			{ scheme: 'untitled', language: '*' }
 		];
+		let notebookProvider = this.registerNotebookProvider();
 		this.extensionContext.subscriptions.push(this.apiWrapper.registerCompletionItemProvider(supportedFileFilter, new NotebookCompletionItemProvider(notebookProvider)));
 
 		return true;
@@ -195,7 +176,7 @@ export class JupyterController implements vscode.Disposable {
 
 	private async handleDependenciesReinstallation(): Promise<void> {
 		if (await this.confirmReinstall()) {
-			this._jupyterInstallation = JupyterServerInstallation.getInstallation(
+			this._jupyterInstallation = await JupyterServerInstallation.getInstallation(
 				this.extensionContext.extensionPath,
 				this.outputChannel,
 				this.apiWrapper,
@@ -225,14 +206,11 @@ export class JupyterController implements vscode.Disposable {
 		}
 	}
 
-	public async doConfigurePython(jupyterInstaller: JupyterServerInstallation): Promise<void> {
-		try {
-			let pythonDialog = new ConfigurePythonDialog(this.appContext, this.outputChannel, jupyterInstaller);
-			await pythonDialog.showDialog();
-		} catch (error) {
-			let message = utils.getErrorMessage(error);
-			this.apiWrapper.showErrorMessage(message);
-		}
+	public doConfigurePython(jupyterInstaller: JupyterServerInstallation): void {
+		let pythonDialog = new ConfigurePythonDialog(this.apiWrapper, this.outputChannel, jupyterInstaller);
+		pythonDialog.showDialog().catch(err => {
+			this.apiWrapper.showErrorMessage(utils.getErrorMessage(err));
+		});
 	}
 
 	public getTextToSendToTerminal(shellType: any): string {

extensions/notebook/src/jupyter/jupyterServerInstallation.ts

@@ -16,7 +16,9 @@ import * as request from 'request';
 import { ApiWrapper } from '../common/apiWrapper';
 import * as constants from '../common/constants';
 import * as utils from '../common/utils';
-import { OutputChannel, ConfigurationTarget, Event, EventEmitter, window } from 'vscode';
+import { OutputChannel, ConfigurationTarget, window } from 'vscode';
+import { Deferred } from '../common/promise';
+import { ConfigurePythonDialog } from '../dialog/configurePythonDialog';
 
 const localize = nls.loadMessageBundle();
 const msgPythonInstallationProgress = localize('msgPythonInstallationProgress', 'Python installation is in progress');
@@ -53,7 +55,7 @@ export default class JupyterServerInstallation {
 
 	private static readonly DefaultPythonLocation = path.join(utils.getUserHome(), 'azuredatastudio-python');
 
-	private _installCompleteEmitter = new EventEmitter<string>();
+	private _installReady: Deferred<void>;
 
 	constructor(extensionPath: string, outputChannel: OutputChannel, apiWrapper: ApiWrapper, pythonInstallationPath?: string, forceInstall?: boolean) {
 		this.extensionPath = extensionPath;
@@ -63,10 +65,15 @@ export default class JupyterServerInstallation {
 		this._forceInstall = !!forceInstall;
 
 		this.configurePackagePaths();
+
+		this._installReady = new Deferred<void>();
+		if (JupyterServerInstallation.isPythonInstalled(this.apiWrapper)) {
+			this._installReady.resolve();
+		}
 	}
 
-	public get onInstallComplete(): Event<string> {
-		return this._installCompleteEmitter.event;
+	public get installReady(): Promise<void> {
+		return this._installReady.promise;
 	}
 
 	public static async getInstallation(
@@ -232,7 +239,7 @@ export default class JupyterServerInstallation {
 		};
 	}
 
-	public async startInstallProcess(pythonInstallationPath?: string): Promise<void> {
+	public startInstallProcess(pythonInstallationPath?: string): Promise<void> {
 		if (pythonInstallationPath) {
 			this._pythonInstallationPath = pythonInstallationPath;
 			this.configurePackagePaths();
@@ -249,23 +256,31 @@ export default class JupyterServerInstallation {
 				operation: op => {
 					this.installDependencies(op)
 						.then(() => {
-							this._installCompleteEmitter.fire();
+							this._installReady.resolve();
 							updateConfig();
 						})
 						.catch(err => {
 							let errorMsg = msgDependenciesInstallationFailed(err);
 							op.updateStatus(azdata.TaskStatus.Failed, errorMsg);
 							this.apiWrapper.showErrorMessage(errorMsg);
-							this._installCompleteEmitter.fire(errorMsg);
+							this._installReady.reject(errorMsg);
 						});
 				}
 			});
 		} else {
 			// Python executable already exists, but the path setting wasn't defined,
 			// so update it here
-			this._installCompleteEmitter.fire();
+			this._installReady.resolve();
 			updateConfig();
 		}
+		return this._installReady.promise;
+	}
+
+	public async promptForPythonInstall(): Promise<void> {
+		if (!JupyterServerInstallation.isPythonInstalled(this.apiWrapper)) {
+			let pythonDialog = new ConfigurePythonDialog(this.apiWrapper, this.outputChannel, this);
+			return pythonDialog.showDialog(true);
+		}
 	}
 
 	private async installJupyterProsePackage(): Promise<void> {

extensions/notebook/src/jupyter/jupyterServerManager.ts

@@ -20,7 +20,7 @@ import { PerNotebookServerInstance, IInstanceOptions } from './serverInstance';
 
 export interface IServerManagerOptions {
 	documentPath: string;
-	jupyterInstallation: Promise<JupyterServerInstallation>;
+	jupyterInstallation: JupyterServerInstallation;
 	extensionContext: vscode.ExtensionContext;
 	apiWrapper?: ApiWrapper;
 	factory?: ServerInstanceFactory;
@@ -62,7 +62,7 @@ export class LocalJupyterServerManager implements nb.ServerManager, vscode.Dispo
 			this._onServerStarted.fire();
 
 		} catch (error) {
-			this.apiWrapper.showErrorMessage(localize('startServerFailed', 'Starting local Notebook server failed with error {0}', utils.getErrorMessage(error)));
+			this.apiWrapper.showErrorMessage(localize('startServerFailed', 'Starting local Notebook server failed with error: {0}', utils.getErrorMessage(error)));
 			throw error;
 		}
 	}
@@ -102,7 +102,8 @@ export class LocalJupyterServerManager implements nb.ServerManager, vscode.Dispo
 	}
 
 	private async doStartServer(): Promise<IServerInstance> {        // We can't find or create servers until the installation is complete
-		let installation = await this.options.jupyterInstallation;
+		let installation = this.options.jupyterInstallation;
+		await installation.promptForPythonInstall();
 
 		// 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

extensions/notebook/src/test/model/serverManager.test.ts

@@ -23,7 +23,7 @@ import { MockExtensionContext } from '../common/stubs';
 describe('Local Jupyter Server Manager', function (): void {
 	let expectedPath = 'my/notebook.ipynb';
 	let serverManager: LocalJupyterServerManager;
-	let deferredInstall: Deferred<JupyterServerInstallation>;
+	let deferredInstall: Deferred<void>;
 	let mockApiWrapper: TypeMoq.IMock<ApiWrapper>;
 	let mockExtensionContext: MockExtensionContext;
 	let mockFactory: TypeMoq.IMock<ServerInstanceFactory>;
@@ -33,10 +33,14 @@ describe('Local Jupyter Server Manager', function (): void {
 		mockApiWrapper.setup(a => a.showErrorMessage(TypeMoq.It.isAny()));
 		mockApiWrapper.setup(a => a.getWorkspacePathFromUri(TypeMoq.It.isAny())).returns(() => undefined);
 		mockFactory = TypeMoq.Mock.ofType(ServerInstanceFactory);
-		deferredInstall = new Deferred<JupyterServerInstallation>();
+
+		deferredInstall = new Deferred<void>();
+		let mockInstall = TypeMoq.Mock.ofType(JupyterServerInstallation, undefined, undefined, '/root');
+		mockInstall.setup(j => j.promptForPythonInstall()).returns(() => deferredInstall.promise);
+
 		serverManager = new LocalJupyterServerManager({
 			documentPath: expectedPath,
-			jupyterInstallation: deferredInstall.promise,
+			jupyterInstallation: mockInstall.object,
 			extensionContext: mockExtensionContext,
 			apiWrapper: mockApiWrapper.object,
 			factory: mockFactory.object
@@ -58,8 +62,8 @@ describe('Local Jupyter Server Manager', function (): void {
 	it('Should configure and start install', async function (): Promise<void> {
 		// Given an install and instance that start with no issues
 		let expectedUri = vscode.Uri.parse('http://localhost:1234?token=abcdefghijk');
-		let [mockInstall, mockServerInstance] = initInstallAndInstance(expectedUri);
-		deferredInstall.resolve(mockInstall.object);
+		let mockServerInstance = initInstallAndInstance(expectedUri);
+		deferredInstall.resolve();
 
 		// When I start the server
 		let notified = false;
@@ -83,9 +87,9 @@ describe('Local Jupyter Server Manager', function (): void {
 	it('Should call stop on server instance', async function (): Promise<void> {
 		// Given an install and instance that start with no issues
 		let expectedUri = vscode.Uri.parse('http://localhost:1234?token=abcdefghijk');
-		let [mockInstall, mockServerInstance] = initInstallAndInstance(expectedUri);
+		let mockServerInstance = initInstallAndInstance(expectedUri);
 		mockServerInstance.setup(s => s.stop()).returns(() => Promise.resolve());
-		deferredInstall.resolve(mockInstall.object);
+		deferredInstall.resolve();
 
 		// When I start and then the server
 		await serverManager.startServer();
@@ -98,9 +102,9 @@ describe('Local Jupyter Server Manager', function (): void {
 	it('Should call stop when extension is disposed', async function (): Promise<void> {
 		// Given an install and instance that start with no issues
 		let expectedUri = vscode.Uri.parse('http://localhost:1234?token=abcdefghijk');
-		let [mockInstall, mockServerInstance] = initInstallAndInstance(expectedUri);
+		let mockServerInstance = initInstallAndInstance(expectedUri);
 		mockServerInstance.setup(s => s.stop()).returns(() => Promise.resolve());
-		deferredInstall.resolve(mockInstall.object);
+		deferredInstall.resolve();
 
 		// When I start and then dispose the extension
 		await serverManager.startServer();
@@ -111,13 +115,12 @@ describe('Local Jupyter Server Manager', function (): void {
 		mockServerInstance.verify(s => s.stop(), TypeMoq.Times.once());
 	});
 
-	function initInstallAndInstance(uri: vscode.Uri): [TypeMoq.IMock<JupyterServerInstallation>, TypeMoq.IMock<IServerInstance>] {
-		let mockInstall = TypeMoq.Mock.ofType(JupyterServerInstallation, undefined, undefined, '/root');
+	function initInstallAndInstance(uri: vscode.Uri): TypeMoq.IMock<IServerInstance> {
 		let mockServerInstance = TypeMoq.Mock.ofType(JupyterServerInstanceStub);
 		mockFactory.setup(f => f.createInstance(TypeMoq.It.isAny())).returns(() => mockServerInstance.object);
 		mockServerInstance.setup(s => s.configure()).returns(() => Promise.resolve());
 		mockServerInstance.setup(s => s.start()).returns(() => Promise.resolve());
 		mockServerInstance.setup(s => s.uri).returns(() => uri);
-		return [mockInstall, mockServerInstance];
+		return mockServerInstance;
 	}
 });