Mitigate (but not fully fix) Run Cell from disconnected notebook (#4960)

This is a partial fix that lays groundwork for full "Prompt to connect" if a kernel needs a connection.
I am waiting on Yurong's refactoring of connection handling before doing any of the prompt work.

- Adds kernel metadata about whether a connection is required.
- For Jupyter, only Spark kernels are listed as requiring a connection
- If this is true and there's no active connection, will show notification and not call execute

In the future, this path will still be used if user is prompted to connect and cancels out.
The future change will be to inject a "connect" handler from notebook.component to the cell callback and use to set connection context
This commit is contained in:
Kevin Cunnane
2019-04-09 17:45:05 -07:00
committed by Karl Burtram
parent 37ba956bad
commit 1fc648ff37
8 changed files with 26 additions and 1 deletions

View File

@@ -63,6 +63,12 @@ export class JupyterKernel implements nb.IKernel {
return true; return true;
} }
public get requiresConnection(): boolean {
// TODO would be good to have a smarter way to do this.
// for now only Spark kernels need a connection
return !!(this.kernelImpl.name && this.kernelImpl.name.toLowerCase().indexOf('spark') > -1);
}
public get isReady(): boolean { public get isReady(): boolean {
return this.kernelImpl.isReady; return this.kernelImpl.isReady;
} }

View File

@@ -4475,6 +4475,7 @@ declare module 'azdata' {
readonly id: string; readonly id: string;
readonly name: string; readonly name: string;
readonly supportsIntellisense: boolean; readonly supportsIntellisense: boolean;
readonly requiresConnection?: boolean;
/** /**
* Test whether the kernel is ready. * Test whether the kernel is ready.
*/ */

View File

@@ -463,6 +463,7 @@ export interface INotebookKernelDetails {
readonly id: string; readonly id: string;
readonly name: string; readonly name: string;
readonly supportsIntellisense: boolean; readonly supportsIntellisense: boolean;
readonly requiresConnection: boolean;
readonly info?: any; readonly info?: any;
} }

View File

@@ -111,7 +111,8 @@ export class ExtHostNotebook implements ExtHostNotebookShape {
id: kernel.id, id: kernel.id,
info: kernel.info, info: kernel.info,
name: kernel.name, name: kernel.name,
supportsIntellisense: kernel.supportsIntellisense supportsIntellisense: kernel.supportsIntellisense,
requiresConnection: kernel.requiresConnection
}; };
return kernelDetails; return kernelDetails;
} }

View File

@@ -352,6 +352,10 @@ class KernelWrapper implements azdata.nb.IKernel {
return this.kernelDetails.supportsIntellisense; return this.kernelDetails.supportsIntellisense;
} }
get requiresConnection(): boolean {
return this.kernelDetails.requiresConnection;
}
get info(): azdata.nb.IInfoReply { get info(): azdata.nb.IInfoReply {
return this._info; return this._info;
} }

View File

@@ -210,6 +210,10 @@ export class CellModel implements ICellModel {
await kernel.interrupt(); await kernel.interrupt();
} else { } else {
// TODO update source based on editor component contents // TODO update source based on editor component contents
if (kernel.requiresConnection && !this.notebookModel.activeConnection) {
this.sendNotification(notificationService, Severity.Error, localize('kernelRequiresConnection', "Please select a connection to run cells for this kernel"));
return false;
}
let content = this.source; let content = this.source;
if (content) { if (content) {
let future = await kernel.requestExecute({ let future = await kernel.requestExecute({

View File

@@ -116,6 +116,10 @@ class EmptyKernel implements nb.IKernel {
return false; return false;
} }
public get requiresConnection(): boolean {
return false;
}
public get isReady(): boolean { public get isReady(): boolean {
return true; return true;
} }

View File

@@ -179,6 +179,10 @@ class SqlKernel extends Disposable implements nb.IKernel {
return true; return true;
} }
public get requiresConnection(): boolean {
return true;
}
public get isReady(): boolean { public get isReady(): boolean {
// should we be checking on the tools service status here? // should we be checking on the tools service status here?
return true; return true;