diff --git a/extensions/notebook/src/jupyter/jupyterServerManager.ts b/extensions/notebook/src/jupyter/jupyterServerManager.ts index f5ae01455c..1dcbf63f91 100644 --- a/extensions/notebook/src/jupyter/jupyterServerManager.ts +++ b/extensions/notebook/src/jupyter/jupyterServerManager.ts @@ -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 is caught and notified up the stack, no longer showing a message here throw error; } } diff --git a/src/sql/parts/notebook/models/clientSession.ts b/src/sql/parts/notebook/models/clientSession.ts index 5497a5ecb2..0070d76ed9 100644 --- a/src/sql/parts/notebook/models/clientSession.ts +++ b/src/sql/parts/notebook/models/clientSession.ts @@ -9,9 +9,9 @@ 'use strict'; import { nb } from 'azdata'; -import * as nls from 'vs/nls'; import { URI } from 'vs/base/common/uri'; import { Event, Emitter } from 'vs/base/common/event'; +import { localize } from 'vs/nls'; import { IClientSession, IKernelPreference, IClientSessionOptions } from './modelInterfaces'; import { Deferred } from 'sql/base/common/promise'; @@ -70,12 +70,14 @@ export class ClientSession implements IClientSession { await this.initializeSession(); await this.updateCachedKernelSpec(); } catch (err) { - this._errorMessage = notebookUtils.getErrorMessage(err); + this._errorMessage = notebookUtils.getErrorMessage(err) || localize('clientSession.unknownError', "An error occurred while starting the notebook session"); } // Always resolving for now. It's up to callers to check for error case this._isReady = true; this._ready.resolve(); - this._kernelChangeCompleted.resolve(); + if (!this.isInErrorState && this._session && this._session.kernel) { + await this.notifyKernelChanged(undefined, this._session.kernel); + } } private async startServer(): Promise { @@ -83,7 +85,7 @@ export class ClientSession implements IClientSession { if (serverManager && !serverManager.isStarted) { await serverManager.startServer(); if (!serverManager.isStarted) { - throw new Error(nls.localize('ServerNotStarted', 'Server did not start for unknown reason')); + throw new Error(localize('ServerNotStarted', "Server did not start for unknown reason")); } this.isServerStarted = serverManager.isStarted; } else { @@ -116,7 +118,7 @@ export class ClientSession implements IClientSession { } catch (err) { // TODO move registration if (err && err.response && err.response.status === 501) { - this.options.notificationService.warn(nls.localize('kernelRequiresConnection', 'Kernel {0} was not found. The default kernel will be used instead.', kernelName)); + this.options.notificationService.warn(localize('kernelRequiresConnection', "Kernel {0} was not found. The default kernel will be used instead.", kernelName)); session = await this.notebookManager.sessionManager.startNew({ path: this.notebookUri.fsPath, kernelName: undefined @@ -246,6 +248,11 @@ export class ClientSession implements IClientSession { this._isReady = kernel.isReady; await this.updateCachedKernelSpec(); // Send resolution events to listeners + await this.notifyKernelChanged(oldKernel, newKernel); + return kernel; + } + + private async notifyKernelChanged(oldKernel: nb.IKernel, newKernel: nb.IKernel): Promise { let changeArgs: nb.IKernelChangedArgs = { oldValue: oldKernel, newValue: newKernel @@ -255,7 +262,6 @@ export class ClientSession implements IClientSession { // Wait on connection configuration to complete before resolving full kernel change this._kernelChangeCompleted.resolve(); this._kernelChangedEmitter.fire(changeArgs); - return kernel; } private async updateCachedKernelSpec(): Promise { diff --git a/src/sql/parts/notebook/models/notebookModel.ts b/src/sql/parts/notebook/models/notebookModel.ts index adfeb5f902..12e6bbbe78 100644 --- a/src/sql/parts/notebook/models/notebookModel.ts +++ b/src/sql/parts/notebook/models/notebookModel.ts @@ -354,7 +354,7 @@ export class NotebookModel extends Disposable implements INotebookModel { cellIndex: index }); } else { - this.notifyError(localize('deleteCellFailed', 'Failed to delete cell.')); + this.notifyError(localize('deleteCellFailed', "Failed to delete cell.")); } } @@ -392,7 +392,7 @@ export class NotebookModel extends Disposable implements INotebookModel { this._onErrorEmitter.fire({ message: error, severity: Severity.Error }); } - public async startSession(manager: INotebookManager, displayName?: string): Promise { + public async startSession(manager: INotebookManager, displayName?: string, setErrorStateOnFail?: boolean): Promise { if (displayName) { let standardKernel = this._notebookOptions.standardKernels.find(kernel => kernel.displayName === displayName); this._defaultKernel = displayName ? { name: standardKernel.name, display_name: standardKernel.displayName } : this._defaultKernel; @@ -406,7 +406,6 @@ export class NotebookModel extends Disposable implements INotebookModel { }); if (!this._activeClientSession) { this.updateActiveClientSession(clientSession); - } let profile = new ConnectionProfile(this._notebookOptions.capabilitiesService, this.connectionProfile); @@ -420,16 +419,31 @@ export class NotebookModel extends Disposable implements INotebookModel { this._activeConnection = undefined; } + clientSession.onKernelChanging(async (e) => { + await this.loadActiveContexts(e); + }); + clientSession.statusChanged(async (session) => { + this._kernelsChangedEmitter.fire(session.kernel); + }); await clientSession.initialize(); // By somehow we have to wait for ready, otherwise may not be called for some cases. await clientSession.ready; - if (clientSession.isInErrorState) { - this.setErrorState(clientSession.errorMessage); - } else { - this._onClientSessionReady.fire(clientSession); - // Once session is loaded, can use the session manager to retrieve useful info - this.loadKernelInfo(clientSession, this.defaultKernel.display_name); + if (clientSession.kernel) { + await clientSession.kernel.ready; + await this.updateKernelInfoOnKernelChange(clientSession.kernel); } + if (clientSession.isInErrorState) { + if (setErrorStateOnFail) { + this.setErrorState(clientSession.errorMessage); + } else { + throw new Error(clientSession.errorMessage); + } + } + this._onClientSessionReady.fire(clientSession); + this._kernelChangedEmitter.fire({ + oldValue: undefined, + newValue: clientSession.kernel + }); } } @@ -552,10 +566,57 @@ export class NotebookModel extends Disposable implements INotebookModel { this.doChangeKernel(displayName, true); } - public async doChangeKernel(displayName: string, mustSetProvider: boolean = true): Promise { - if (mustSetProvider) { - await this.setProviderIdAndStartSession(displayName); + private async doChangeKernel(displayName: string, mustSetProvider: boolean = true, restoreOnFail: boolean = true): Promise { + let oldDisplayName = this._activeClientSession && this._activeClientSession.kernel ? this._activeClientSession.kernel.name : undefined; + try { + let changeKernelNeeded = true; + if (mustSetProvider) { + changeKernelNeeded = await this.setProviderIdAndStartSession(displayName); + } + if (changeKernelNeeded) { + let spec = this.findSpec(displayName); + if (this._activeClientSession && this._activeClientSession.isReady) { + let kernel = await this._activeClientSession.changeKernel(spec, this._oldKernel); + try { + await kernel.ready; + await this.updateKernelInfoOnKernelChange(kernel); + } catch (err2) { + // TODO should we handle this in any way? + console.log(`doChangeKernel: ignoring error ${notebookUtils.getErrorMessage(err2)}`); + } + } + } + } catch (err) { + if (oldDisplayName && restoreOnFail) { + this.notifyError(localize('changeKernelFailedRetry', "Failed to change kernel. Kernel {0} will be used. Error was: {1}", oldDisplayName, notebookUtils.getErrorMessage(err))); + // Clear out previous kernel + let failedProviderId = this.tryFindProviderForKernel(displayName, true); + let oldProviderId = this.tryFindProviderForKernel(oldDisplayName, true); + if (failedProviderId !== oldProviderId) { + // We need to clear out the old kernel information so we switch providers. Otherwise in the SQL -> Jupyter -> SQL failure case, + // we would never reset the providers + this._oldKernel = undefined; + } + return this.doChangeKernel(oldDisplayName, mustSetProvider, false); + } else { + this.notifyError(localize('changeKernelFailed', "Failed to change kernel due to error: {0}", notebookUtils.getErrorMessage(err))); + this._kernelChangedEmitter.fire({ + newValue: undefined, + oldValue: undefined + }); + } } + // Else no need to do anything + } + + private async updateKernelInfoOnKernelChange(kernel: nb.IKernel) { + await this.updateKernelInfo(kernel); + if (kernel.info) { + this.updateLanguageInfo(kernel.info.language_info); + } + } + + private findSpec(displayName: string) { let spec = this.getKernelSpecFromDisplayName(displayName); if (spec) { // Ensure that the kernel we try to switch to is a valid kernel; if not, use the default @@ -563,24 +624,11 @@ export class NotebookModel extends Disposable implements INotebookModel { if (kernelSpecs && kernelSpecs.length > 0 && kernelSpecs.findIndex(k => k.display_name === spec.display_name) < 0) { spec = kernelSpecs.find(spec => spec.name === this.notebookManager.sessionManager.specs.defaultKernel); } - } else { + } + else { spec = notebookConstants.sqlKernelSpec; } - if (this._activeClientSession && this._activeClientSession.isReady) { - return this._activeClientSession.changeKernel(spec, this._oldKernel) - .then((kernel) => { - this.updateKernelInfo(kernel); - kernel.ready.then(() => { - if (kernel.info) { - this.updateLanguageInfo(kernel.info.language_info); - } - }, err => undefined); - }).catch((err) => { - this.notifyError(localize('changeKernelFailed', 'Failed to change kernel: {0}', notebookUtils.getErrorMessage(err))); - // TODO should revert kernels dropdown - }); - } - return Promise.resolve(); + return spec; } public async changeContext(server: string, newConnection?: IConnectionProfile, hideErrorMessage?: boolean): Promise { @@ -613,7 +661,7 @@ export class NotebookModel extends Disposable implements INotebookModel { }); } catch (err) { let msg = notebookUtils.getErrorMessage(err); - this.notifyError(localize('changeContextFailed', 'Changing context failed: {0}', msg)); + this.notifyError(localize('changeContextFailed', "Changing context failed: {0}", msg)); } } @@ -631,17 +679,6 @@ export class NotebookModel extends Disposable implements INotebookModel { } } - private loadKernelInfo(clientSession: IClientSession, displayName: string): void { - clientSession.onKernelChanging(async (e) => { - await this.loadActiveContexts(e); - }); - clientSession.statusChanged(async (session) => { - this._kernelsChangedEmitter.fire(session.kernel); - }); - - this.doChangeKernel(displayName, false); - } - // Get default language if saved in notebook file // Otherwise, default to python private getDefaultLanguageInfo(notebook: nb.INotebookContents): nb.ILanguageInfo { @@ -703,7 +740,7 @@ export class NotebookModel extends Disposable implements INotebookModel { private setErrorState(errMsg: string): void { this._inErrorState = true; - let msg = localize('startSessionFailed', 'Could not start session: {0}', errMsg); + let msg = localize('startSessionFailed', "Could not start session: {0}", errMsg); this.notifyError(msg); } @@ -730,7 +767,7 @@ export class NotebookModel extends Disposable implements INotebookModel { } await this.shutdownActiveSession(); } catch (err) { - this.notifyError(localize('shutdownError', 'An error occurred when closing the notebook: {0}', err)); + this.notifyError(localize('shutdownError', "An error occurred when closing the notebook: {0}", err)); } } @@ -740,7 +777,7 @@ export class NotebookModel extends Disposable implements INotebookModel { await this._activeClientSession.ready; } catch (err) { - this.notifyError(localize('shutdownClientSessionError', 'A client session error occurred when closing the notebook: {0}', err)); + this.notifyError(localize('shutdownClientSessionError', "A client session error occurred when closing the notebook: {0}", err)); } await this._activeClientSession.shutdown(); this.clearClientSessionListeners(); @@ -794,46 +831,39 @@ export class NotebookModel extends Disposable implements INotebookModel { * Set _providerId and start session if it is new provider * @param displayName Kernel dispay name */ - private async setProviderIdAndStartSession(displayName: string): Promise { + private async setProviderIdAndStartSession(displayName: string): Promise { if (displayName) { if (this._activeClientSession && this._activeClientSession.isReady) { this._oldKernel = this._activeClientSession.kernel; - let providerId = this.tryFindProviderForKernel(displayName); + } + let providerId = this.tryFindProviderForKernel(displayName); - if (providerId) { - if (providerId !== this._providerId) { - this._providerId = providerId; - this._onProviderIdChanged.fire(this._providerId); + if (providerId && providerId !== this._providerId) { + this._providerId = providerId; + this._onProviderIdChanged.fire(this._providerId); - await this.shutdownActiveSession(); - - try { - let manager = this.getNotebookManager(providerId); - if (manager) { - await this.startSession(manager, displayName); - } else { - throw new Error(localize('ProviderNoManager', "Can't find notebook manager for provider {0}", providerId)); - } - } - catch (err) { - console.log(err); - } - } else { - console.log(`No provider found supporting the kernel: ${displayName}`); - } + await this.shutdownActiveSession(); + let manager = this.getNotebookManager(providerId); + if (manager) { + await this.startSession(manager, displayName, false); + } else { + throw new Error(localize('ProviderNoManager', "Can't find notebook manager for provider {0}", providerId)); } + return true; } } + return false; } - private tryFindProviderForKernel(displayName: string) { + private tryFindProviderForKernel(displayName: string, alwaysReturnId: boolean = false) { if (!displayName) { return undefined; } let standardKernel = this.getStandardKernelFromDisplayName(displayName); - if (standardKernel && this._oldKernel && this._oldKernel.name !== standardKernel.name) { - if (this._kernelDisplayNameToNotebookProviderIds.has(displayName)) { - return this._kernelDisplayNameToNotebookProviderIds.get(displayName); + if (standardKernel) { + let providerId = this._kernelDisplayNameToNotebookProviderIds.get(displayName); + if (alwaysReturnId || (!this._oldKernel || this._oldKernel.name !== standardKernel.name)) { + return providerId; } } return undefined; @@ -883,7 +913,7 @@ export class NotebookModel extends Disposable implements INotebookModel { }; } - onCellChange(cell: CellModel, change: NotebookChangeType): void { + onCellChange(cell: ICellModel, change: NotebookChangeType): void { let changeInfo: NotebookContentChange = { changeType: change, cells: [cell] diff --git a/src/sql/parts/notebook/notebook.component.ts b/src/sql/parts/notebook/notebook.component.ts index 61b6f8a0d8..801a1876ff 100644 --- a/src/sql/parts/notebook/notebook.component.ts +++ b/src/sql/parts/notebook/notebook.component.ts @@ -248,7 +248,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe this._model = this._register(model); this.updateToolbarComponents(this._model.trustedMode); this._modelRegisteredDeferred.resolve(this._model); - await model.startSession(this.model.notebookManager); + await model.startSession(this.model.notebookManager, undefined, true); this.detectChanges(); } diff --git a/src/sql/parts/notebook/notebookActions.ts b/src/sql/parts/notebook/notebookActions.ts index 117d369f99..65441f97fe 100644 --- a/src/sql/parts/notebook/notebookActions.ts +++ b/src/sql/parts/notebook/notebookActions.ts @@ -235,18 +235,24 @@ export class KernelsDropdown extends SelectBox { this._register(this.model.kernelChanged((changedArgs: azdata.nb.IKernelChangedArgs) => { this.updateKernel(changedArgs.newValue); })); + let kernel = this.model.clientSession && this.model.clientSession.kernel; + this.updateKernel(kernel); } // Update SelectBox values public updateKernel(kernel: azdata.nb.IKernel) { - if (kernel) { + let kernels: string[] = this.model.standardKernelsDisplayName(); + if (kernel && kernel.isReady) { let standardKernel = this.model.getStandardKernelFromName(kernel.name); - let kernels: string[] = this.model.standardKernelsDisplayName(); if (kernels && standardKernel) { let index = kernels.findIndex((kernel => kernel === standardKernel.displayName)); this.setOptions(kernels, index); } + } else if (this.model.clientSession.isInErrorState) { + let noKernelName = localize('noKernel', "No Kernel"); + kernels.unshift(noKernelName); + this.setOptions(kernels, 0); } } @@ -283,22 +289,26 @@ export class AttachToDropdown extends SelectBox { public updateModel(model: INotebookModel): void { this.model = model as NotebookModel; this._register(model.contextsChanged(() => { - let kernelDisplayName: string = this.getKernelDisplayName(); - if (kernelDisplayName) { - this.loadAttachToDropdown(this.model, kernelDisplayName); - } + this.handleContextsChanged(); })); this._register(this.model.contextsLoading(() => { this.setOptions([msgLoadingContexts], 0); })); + this.handleContextsChanged(); + } + + private handleContextsChanged(showSelectConnection?: boolean) { + let kernelDisplayName: string = this.getKernelDisplayName(); + if (kernelDisplayName) { + this.loadAttachToDropdown(this.model, kernelDisplayName, showSelectConnection); + } else if (this.model.clientSession.isInErrorState) { + this.setOptions([localize('noContextAvailable', "None")], 0); + } } private updateAttachToDropdown(model: INotebookModel): void { model.onValidConnectionSelected(validConnection => { - let kernelDisplayName: string = this.getKernelDisplayName(); - if (kernelDisplayName) { - this.loadAttachToDropdown(this.model, kernelDisplayName, !validConnection); - } + this.handleContextsChanged(!validConnection); }); } diff --git a/src/sql/workbench/api/node/extHostNotebook.ts b/src/sql/workbench/api/node/extHostNotebook.ts index 1e72fa3f57..0b9d2750b5 100644 --- a/src/sql/workbench/api/node/extHostNotebook.ts +++ b/src/sql/workbench/api/node/extHostNotebook.ts @@ -81,23 +81,27 @@ export class ExtHostNotebook implements ExtHostNotebookShape { $startNewSession(managerHandle: number, options: azdata.nb.ISessionOptions): Thenable { return this._withSessionManager(managerHandle, async (sessionManager) => { - let session = await sessionManager.startNew(options); - let sessionId = this._addNewAdapter(session); - let kernelDetails: INotebookKernelDetails = undefined; - if (session.kernel) { - kernelDetails = this.saveKernel(session.kernel); + try { + let session = await sessionManager.startNew(options); + let sessionId = this._addNewAdapter(session); + let kernelDetails: INotebookKernelDetails = undefined; + if (session.kernel) { + kernelDetails = this.saveKernel(session.kernel); + } + let details: INotebookSessionDetails = { + sessionId: sessionId, + id: session.id, + path: session.path, + name: session.name, + type: session.type, + status: session.status, + canChangeKernels: session.canChangeKernels, + kernelDetails: kernelDetails + }; + return details; + } catch (error) { + throw typeof(error) === 'string' ? new Error(error) : error; } - let details: INotebookSessionDetails = { - sessionId: sessionId, - id: session.id, - path: session.path, - name: session.name, - type: session.type, - status: session.status, - canChangeKernels: session.canChangeKernels, - kernelDetails: kernelDetails - }; - return details; }); } @@ -267,7 +271,16 @@ export class ExtHostNotebook implements ExtHostNotebookShape { if (manager === undefined) { return TPromise.wrapError(new Error(localize('errNoManager', 'No Manager found'))); } - return TPromise.wrap(callback(manager)); + return TPromise.wrap(this.callbackWithErrorWrap(callback, manager)); + } + + private async callbackWithErrorWrap(callback: (manager: NotebookManagerAdapter) => R | PromiseLike, manager: NotebookManagerAdapter): Promise { + try { + let value = await callback(manager); + return value; + } catch (error) { + throw typeof(error) === 'string' ? new Error(error) : error; + } } private _withServerManager(handle: number, callback: (manager: azdata.nb.ServerManager) => R | PromiseLike): TPromise {