From 0bc3716f74cf2365a85aa24f09a1b5a65f3e56c0 Mon Sep 17 00:00:00 2001 From: Kevin Cunnane Date: Thu, 14 Mar 2019 15:49:14 -0700 Subject: [PATCH] FIX #4513 Notebook stuck at changing kernel (#4518) * FIX #4513 Notebook stuck at changing kernel - Intra-provider kernel change didn't happen because we only tried changing kernel on new session creation. - Inverted the logic (e.g. did the right thing) and renamed the method so it's clearer what we're doing & what the boolean value should be - Manually tested all the known scenarios --- src/sql/parts/notebook/models/notebookModel.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/sql/parts/notebook/models/notebookModel.ts b/src/sql/parts/notebook/models/notebookModel.ts index 12e6bbbe78..b575303c57 100644 --- a/src/sql/parts/notebook/models/notebookModel.ts +++ b/src/sql/parts/notebook/models/notebookModel.ts @@ -567,11 +567,17 @@ export class NotebookModel extends Disposable implements INotebookModel { } private async doChangeKernel(displayName: string, mustSetProvider: boolean = true, restoreOnFail: boolean = true): Promise { + if (!displayName) { + // Can't change to an undefined kernel + return; + } let oldDisplayName = this._activeClientSession && this._activeClientSession.kernel ? this._activeClientSession.kernel.name : undefined; try { let changeKernelNeeded = true; if (mustSetProvider) { - changeKernelNeeded = await this.setProviderIdAndStartSession(displayName); + let providerChanged = await this.tryStartSessionByChangingProviders(displayName); + // If provider was changed, a new session with new kernel is already created. We can skip calling changeKernel. + changeKernelNeeded = !providerChanged; } if (changeKernelNeeded) { let spec = this.findSpec(displayName); @@ -831,7 +837,7 @@ 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 tryStartSessionByChangingProviders(displayName: string): Promise { if (displayName) { if (this._activeClientSession && this._activeClientSession.isReady) { this._oldKernel = this._activeClientSession.kernel; @@ -855,7 +861,7 @@ export class NotebookModel extends Disposable implements INotebookModel { return false; } - private tryFindProviderForKernel(displayName: string, alwaysReturnId: boolean = false) { + private tryFindProviderForKernel(displayName: string, alwaysReturnId: boolean = false): string { if (!displayName) { return undefined; }