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
This commit is contained in:
Kevin Cunnane
2019-03-14 15:49:14 -07:00
committed by GitHub
parent ca23ea0f69
commit 0bc3716f74

View File

@@ -567,11 +567,17 @@ export class NotebookModel extends Disposable implements INotebookModel {
} }
private async doChangeKernel(displayName: string, mustSetProvider: boolean = true, restoreOnFail: boolean = true): Promise<void> { private async doChangeKernel(displayName: string, mustSetProvider: boolean = true, restoreOnFail: boolean = true): Promise<void> {
if (!displayName) {
// Can't change to an undefined kernel
return;
}
let oldDisplayName = this._activeClientSession && this._activeClientSession.kernel ? this._activeClientSession.kernel.name : undefined; let oldDisplayName = this._activeClientSession && this._activeClientSession.kernel ? this._activeClientSession.kernel.name : undefined;
try { try {
let changeKernelNeeded = true; let changeKernelNeeded = true;
if (mustSetProvider) { 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) { if (changeKernelNeeded) {
let spec = this.findSpec(displayName); 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 * Set _providerId and start session if it is new provider
* @param displayName Kernel dispay name * @param displayName Kernel dispay name
*/ */
private async setProviderIdAndStartSession(displayName: string): Promise<boolean> { private async tryStartSessionByChangingProviders(displayName: string): Promise<boolean> {
if (displayName) { if (displayName) {
if (this._activeClientSession && this._activeClientSession.isReady) { if (this._activeClientSession && this._activeClientSession.isReady) {
this._oldKernel = this._activeClientSession.kernel; this._oldKernel = this._activeClientSession.kernel;
@@ -855,7 +861,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
return false; return false;
} }
private tryFindProviderForKernel(displayName: string, alwaysReturnId: boolean = false) { private tryFindProviderForKernel(displayName: string, alwaysReturnId: boolean = false): string {
if (!displayName) { if (!displayName) {
return undefined; return undefined;
} }