diff --git a/extensions/integration-tests/src/test/notebook.util.ts b/extensions/integration-tests/src/test/notebook.util.ts index 6f237f4ada..fec09f6830 100644 --- a/extensions/integration-tests/src/test/notebook.util.ts +++ b/extensions/integration-tests/src/test/notebook.util.ts @@ -24,9 +24,9 @@ export const pySparkNotebookContent: azdata.nb.INotebookContents = { execution_count: 1 }], metadata: { - 'kernelspec': { - 'name': 'pysparkkernel', - 'display_name': 'PySpark' + kernelspec: { + name: 'pysparkkernel', + display_name: 'PySpark' } }, nbformat: 4, @@ -41,8 +41,9 @@ export const notebookContentForCellLanguageTest: azdata.nb.INotebookContents = { execution_count: 1 }], metadata: { - 'kernelspec': { - 'name': '' + kernelspec: { + name: '', + display_name: '' }, }, nbformat: 4, @@ -72,9 +73,9 @@ export const pythonNotebookMultipleCellsContent: azdata.nb.INotebookContents = { execution_count: 1 }], metadata: { - 'kernelspec': { - 'name': 'python3', - 'display_name': 'Python 3' + kernelspec: { + name: 'python3', + display_name: 'Python 3' } }, nbformat: 4, @@ -89,9 +90,9 @@ export const sqlNotebookContent: azdata.nb.INotebookContents = { execution_count: 1 }], metadata: { - 'kernelspec': { - 'name': 'SQL', - 'display_name': 'SQL' + kernelspec: { + name: 'SQL', + display_name: 'SQL' } }, nbformat: 4, @@ -116,9 +117,9 @@ export const sqlNotebookMultipleCellsContent: azdata.nb.INotebookContents = { execution_count: 1 }], metadata: { - 'kernelspec': { - 'name': 'SQL', - 'display_name': 'SQL' + kernelspec: { + name: 'SQL', + display_name: 'SQL' } }, nbformat: 4, @@ -126,9 +127,9 @@ export const sqlNotebookMultipleCellsContent: azdata.nb.INotebookContents = { }; export const pySparkKernelMetadata = { - 'kernelspec': { - 'name': 'pysparkkernel', - 'display_name': 'PySpark' + kernelspec: { + name: 'pysparkkernel', + display_name: 'PySpark' } }; @@ -138,9 +139,9 @@ export const pySparkKernelSpec = { }; export const sqlKernelMetadata = { - 'kernelspec': { - 'name': 'SQL', - 'display_name': 'SQL' + kernelspec: { + name: 'SQL', + display_name: 'SQL' } }; @@ -150,9 +151,9 @@ export const sqlKernelSpec: azdata.nb.IKernelSpec = { }; export const pythonKernelMetadata = { - 'kernelspec': { - 'name': 'python3', - 'display_name': 'Python 3' + kernelspec: { + name: 'python3', + display_name: 'Python 3' } }; diff --git a/extensions/notebook/src/test/model/sessionManager.test.ts b/extensions/notebook/src/test/model/sessionManager.test.ts index 3648e31320..60e00f786d 100644 --- a/extensions/notebook/src/test/model/sessionManager.test.ts +++ b/extensions/notebook/src/test/model/sessionManager.test.ts @@ -186,7 +186,8 @@ describe('Jupyter Session', function (): void { // When I call changeKernel on the wrapper let kernel = await session.changeKernel({ - name: 'python' + name: 'python', + display_name: 'Python' }); // Then I expect it to have the ID, and only be called once should(kernel.id).equal('id'); diff --git a/src/sql/azdata.d.ts b/src/sql/azdata.d.ts index 5eace26bd1..f8059ddfa6 100644 --- a/src/sql/azdata.d.ts +++ b/src/sql/azdata.d.ts @@ -4653,11 +4653,14 @@ declare module 'azdata' { } export interface INotebookMetadata { - kernelspec: IKernelInfo; + kernelspec?: IKernelInfo | IKernelSpec; language_info?: ILanguageInfo; tags?: string[]; } + /** + * @deprecated Use IKernelSpec instead + */ export interface IKernelInfo { name: string; language?: string; @@ -5059,8 +5062,8 @@ declare module 'azdata' { * An arguments object for the kernel changed event. */ export interface IKernelChangedArgs { - oldValue: IKernel | null; - newValue: IKernel | null; + oldValue: IKernel | undefined; + newValue: IKernel | undefined; } /// -------- JSON objects, and objects primarily intended not to have methods ----------- @@ -5088,7 +5091,7 @@ declare module 'azdata' { /** * The original outgoing message. */ - readonly msg: IMessage; + readonly msg: IMessage | undefined; /** * A Thenable that resolves when the future is done. @@ -5183,7 +5186,7 @@ declare module 'azdata' { */ export interface IExecuteReply { status: 'ok' | 'error' | 'abort'; - execution_count: number | null; + execution_count: number | null | undefined; } /** @@ -5199,11 +5202,11 @@ declare module 'azdata' { * **See also:** [[IMessage]] */ export interface IHeader { - username: string; - version: string; - session: string; - msg_id: string; msg_type: string; + username?: string; + version?: string; + session?: string; + msg_id?: string; } /** @@ -5211,10 +5214,10 @@ declare module 'azdata' { */ export interface IMessage { type: Channel; - header: IHeader; - parent_header: IHeader | {}; - metadata: {}; content: any; + header?: IHeader; + parent_header?: IHeader | {}; + metadata?: {}; } /** diff --git a/src/sql/workbench/api/common/sqlExtHost.protocol.ts b/src/sql/workbench/api/common/sqlExtHost.protocol.ts index 63ae3f1d66..93ac7c8749 100644 --- a/src/sql/workbench/api/common/sqlExtHost.protocol.ts +++ b/src/sql/workbench/api/common/sqlExtHost.protocol.ts @@ -936,7 +936,7 @@ export interface MainThreadNotebookDocumentsAndEditorsShape extends IDisposable $runAllCells(id: string, startCellUri?: UriComponents, endCellUri?: UriComponents): Promise; $clearOutput(id: string, cellUri: UriComponents): Promise; $clearAllOutputs(id: string): Promise; - $changeKernel(id: string, kernel: azdata.nb.IKernelInfo): Promise; + $changeKernel(id: string, kernel: azdata.nb.IKernelSpec): Promise; $registerNavigationProvider(providerId: string, handle: number); } diff --git a/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts b/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts index 7cc9d398f9..5b87693eb5 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts @@ -188,7 +188,8 @@ export async function createandLoadNotebookModel(codeContent?: nb.INotebookConte metadata: { kernelspec: { name: 'python', - language: 'python' + language: 'python', + display_name: 'Python 3' } }, nbformat: 4, diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/clientSession.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/clientSession.test.ts index d38d499ed4..d95f795b3b 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/clientSession.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/clientSession.test.ts @@ -49,7 +49,7 @@ suite('Client Session', function (): void { assert(!session.isReady); assert.equal(session.status, 'starting'); assert(!session.isInErrorState); - assert(isUndefinedOrNull(session.errorMessage)); + assert.equal(session.errorMessage, ''); }); test('Should call on serverManager startup if set', async function (): Promise { diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/contentManagers.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/contentManagers.test.ts index 7aaecb149c..901a3ffc84 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/contentManagers.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/contentManagers.test.ts @@ -27,7 +27,8 @@ let expectedNotebookContent: nb.INotebookContents = { metadata: { kernelspec: { name: 'mssql', - language: 'sql' + language: 'sql', + display_name: 'SQL' } }, nbformat: 4, @@ -117,7 +118,8 @@ suite('Local Content Manager', function (): void { metadata: { kernelspec: { name: 'mssql', - language: 'sql' + language: 'sql', + display_name: 'SQL' } }, nbformat: 4, @@ -158,7 +160,8 @@ suite('Local Content Manager', function (): void { metadata: { kernelspec: { name: 'mssql', - language: 'sql' + language: 'sql', + display_name: 'SQL' } }, nbformat: 4, @@ -191,7 +194,8 @@ suite('Local Content Manager', function (): void { metadata: { kernelspec: { name: 'Python 3', - language: 'python3' + language: 'python3', + display_name: 'Python 3' } }, nbformat: 4, diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts index e0ffd693e3..a39d908e35 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookFindModel.test.ts @@ -46,7 +46,8 @@ let expectedNotebookContent: nb.INotebookContents = { metadata: { kernelspec: { name: 'mssql', - language: 'sql' + language: 'sql', + display_name: 'SQL' } }, nbformat: 4, @@ -184,7 +185,8 @@ suite('Notebook Find Model', function (): void { metadata: { kernelspec: { name: 'mssql', - language: 'sql' + language: 'sql', + display_name: 'SQL' } }, nbformat: 4, @@ -216,7 +218,8 @@ suite('Notebook Find Model', function (): void { metadata: { kernelspec: { name: 'python', - language: 'python' + language: 'python', + display_name: 'Python' } }, nbformat: 4, @@ -241,7 +244,8 @@ suite('Notebook Find Model', function (): void { metadata: { kernelspec: { name: 'python', - language: 'python' + language: 'python', + display_name: 'Python' } }, nbformat: 4, @@ -301,7 +305,8 @@ suite('Notebook Find Model', function (): void { metadata: { kernelspec: { name: 'python', - language: 'python' + language: 'python', + display_name: 'Python' } }, nbformat: 4, @@ -333,7 +338,8 @@ suite('Notebook Find Model', function (): void { metadata: { kernelspec: { name: 'python', - language: 'python' + language: 'python', + display_name: 'Python' } }, nbformat: 4, @@ -364,7 +370,8 @@ suite('Notebook Find Model', function (): void { metadata: { kernelspec: { name: 'mssql', - language: 'sql' + language: 'sql', + display_name: 'SQL' } }, nbformat: 4, diff --git a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts index 6d78eef77f..127d916bc3 100644 --- a/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/electron-browser/notebookModel.test.ts @@ -52,7 +52,8 @@ let expectedNotebookContent: nb.INotebookContents = { metadata: { kernelspec: { name: 'mssql', - language: 'sql' + language: 'sql', + display_name: 'SQL' } }, nbformat: 4, @@ -69,7 +70,8 @@ let expectedNotebookContentOneCell: nb.INotebookContents = { metadata: { kernelspec: { name: 'mssql', - language: 'sql' + language: 'sql', + display_name: 'SQL' } }, nbformat: 4, @@ -140,7 +142,8 @@ suite('notebook model', function (): void { metadata: { kernelspec: { name: 'mssql', - language: 'sql' + language: 'sql', + display_name: 'SQL' } }, nbformat: 4, diff --git a/src/sql/workbench/contrib/notebook/test/stubs.ts b/src/sql/workbench/contrib/notebook/test/stubs.ts index 500db2f2e3..9070095de9 100644 --- a/src/sql/workbench/contrib/notebook/test/stubs.ts +++ b/src/sql/workbench/contrib/notebook/test/stubs.ts @@ -52,7 +52,7 @@ export class NotebookModelStub implements INotebookModel { get kernelChanged(): vsEvent.Event { throw new Error('method not implemented.'); } - get kernelsChanged(): vsEvent.Event { + get kernelsChanged(): vsEvent.Event { throw new Error('method not implemented.'); } get layoutChanged(): vsEvent.Event { diff --git a/src/sql/workbench/services/notebook/browser/models/cell.ts b/src/sql/workbench/services/notebook/browser/models/cell.ts index 3097e202be..302c634848 100644 --- a/src/sql/workbench/services/notebook/browser/models/cell.ts +++ b/src/sql/workbench/services/notebook/browser/models/cell.ts @@ -475,14 +475,12 @@ export class CellModel extends Disposable implements ICellModel { await clientSession.kernelChangeCompleted; } if (!clientSession.kernel) { - let defaultKernel = model && model.defaultKernel && model.defaultKernel.name; + let defaultKernel = model && model.defaultKernel; if (!defaultKernel) { this.sendNotification(notificationService, Severity.Error, localize('noDefaultKernel', "No kernel is available for this notebook")); return undefined; } - await clientSession.changeKernel({ - name: defaultKernel - }); + await clientSession.changeKernel(defaultKernel); } return clientSession.kernel; } diff --git a/src/sql/workbench/services/notebook/browser/models/clientSession.ts b/src/sql/workbench/services/notebook/browser/models/clientSession.ts index f5ca462cea..819d6f180f 100644 --- a/src/sql/workbench/services/notebook/browser/models/clientSession.ts +++ b/src/sql/workbench/services/notebook/browser/models/clientSession.ts @@ -30,22 +30,22 @@ export class ClientSession implements IClientSession { private _unhandledMessageEmitter = new Emitter(); private _propertyChangedEmitter = new Emitter<'path' | 'name' | 'type'>(); private _notebookUri: URI; - private _type: string; - private _name: string; + private _type: string = ''; + private _name: string = ''; private _isReady: boolean; private _ready: Deferred; private _kernelChangeCompleted: Deferred; - private _kernelDisplayName: string; - private _errorMessage: string; - private _cachedKernelSpec: nb.IKernelSpec; + private _kernelDisplayName: string = ''; + private _errorMessage: string = ''; + private _cachedKernelSpec: nb.IKernelSpec | undefined; private _kernelChangeHandlers: KernelChangeHandler[] = []; private _defaultKernel: nb.IKernelSpec; //#endregion - private _serverLoadFinished: Promise; - private _session: nb.ISession; - private isServerStarted: boolean; + private _serverLoadFinished: Promise = Promise.resolve(); + private _session: nb.ISession | undefined; + private isServerStarted: boolean = false; private notebookManager: INotebookManager; private _kernelConfigActions: ((kernelName: string) => Promise)[] = []; private _connectionId: string = ''; @@ -72,7 +72,7 @@ export class ClientSession implements IClientSession { this._isReady = true; this._ready.resolve(); if (!this.isInErrorState && this._session && this._session.kernel) { - await this.notifyKernelChanged(undefined, this._session.kernel); + await this.notifyKernelChanged(this._session.kernel); } } @@ -173,7 +173,7 @@ export class ClientSession implements IClientSession { public get propertyChanged(): Event<'path' | 'name' | 'type'> { return this._propertyChangedEmitter.event; } - public get kernel(): nb.IKernel | null { + public get kernel(): nb.IKernel | undefined { return this._session ? this._session.kernel : undefined; } public get notebookUri(): URI { @@ -210,7 +210,7 @@ export class ClientSession implements IClientSession { return !!this._errorMessage; } - public get cachedKernelSpec(): nb.IKernelSpec { + public get cachedKernelSpec(): nb.IKernelSpec | undefined { return this._cachedKernelSpec; } //#endregion @@ -219,29 +219,31 @@ export class ClientSession implements IClientSession { /** * Change the current kernel associated with the document. */ - async changeKernel(options: nb.IKernelSpec, oldValue?: nb.IKernel): Promise { + async changeKernel(options: nb.IKernelSpec, oldValue?: nb.IKernel): Promise { this._kernelChangeCompleted = new Deferred(); this._isReady = false; let oldKernel = oldValue ? oldValue : this.kernel; let kernel = await this.doChangeKernel(options); try { - await kernel.ready; + await kernel?.ready; } catch (error) { // Cleanup some state before re-throwing - this._isReady = kernel.isReady; + this._isReady = kernel ? kernel.isReady : false; this._kernelChangeCompleted.resolve(); throw error; } - let newKernel = this._session ? kernel : this._session.kernel; - this._isReady = kernel.isReady; + let newKernel = this._session ? this._session.kernel : kernel; + this._isReady = kernel ? kernel.isReady : false; await this.updateCachedKernelSpec(); // Send resolution events to listeners - await this.notifyKernelChanged(oldKernel, newKernel); + if (newKernel) { + await this.notifyKernelChanged(newKernel, oldKernel); + } return kernel; } - private async notifyKernelChanged(oldKernel: nb.IKernel, newKernel: nb.IKernel): Promise { + private async notifyKernelChanged(newKernel: nb.IKernel, oldKernel?: nb.IKernel): Promise { let changeArgs: nb.IKernelChangedArgs = { oldValue: oldKernel, newValue: newKernel @@ -267,8 +269,8 @@ export class ClientSession implements IClientSession { /** * Helper method to either call ChangeKernel on current session, or start a new session */ - private async doChangeKernel(options: nb.IKernelSpec): Promise { - let kernel: nb.IKernel; + private async doChangeKernel(options: nb.IKernelSpec): Promise { + let kernel: nb.IKernel | undefined; if (this._session) { kernel = await this._session.changeKernel(options); await this.runKernelConfigActions(kernel.name); @@ -289,7 +291,7 @@ export class ClientSession implements IClientSession { // TODO is there any case where skipping causes errors? So far it seems like it gets called twice return; } - if (connection.id !== '-1' && connection.id !== this._connectionId) { + if (connection.id !== '-1' && connection.id !== this._connectionId && this._session) { await this._session.configureConnection(connection); this._connectionId = connection.id; } diff --git a/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts b/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts index fa0f2af0c6..1ebd233fef 100644 --- a/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts +++ b/src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts @@ -82,7 +82,7 @@ export interface IClientSession extends IDisposable { /** * The current kernel associated with the document. */ - readonly kernel: nb.IKernel | null; + readonly kernel: nb.IKernel | undefined; /** * The current path associated with the client session. @@ -133,7 +133,7 @@ export interface IClientSession extends IDisposable { */ readonly kernelDisplayName: string; - readonly cachedKernelSpec: nb.IKernelSpec; + readonly cachedKernelSpec: nb.IKernelSpec | undefined; /** * Initializes the ClientSession, by starting the server and @@ -149,7 +149,7 @@ export interface IClientSession extends IDisposable { changeKernel( options: nb.IKernelSpec, oldKernel?: nb.IKernel - ): Promise; + ): Promise; /** * Configure the current kernel associated with the document. @@ -222,7 +222,7 @@ export interface INotebookModel { /** * Cell List for this model */ - readonly cells: ReadonlyArray; + readonly cells: ReadonlyArray | undefined; /** * The active cell for this model. May be undefined @@ -232,7 +232,7 @@ export interface INotebookModel { /** * Client Session in the notebook, used for sending requests to the notebook service */ - readonly clientSession: IClientSession; + readonly clientSession: IClientSession | undefined; /** * Promise indicating when client session is ready to use. */ @@ -244,7 +244,7 @@ export interface INotebookModel { /** * LanguageInfo saved in the notebook */ - readonly languageInfo: nb.ILanguageInfo; + readonly languageInfo: nb.ILanguageInfo | undefined; /** * Current default language for the notebook */ @@ -270,7 +270,7 @@ export interface INotebookModel { * Event fired on first initialization of the kernels and * on subsequent change events */ - readonly kernelsChanged: Event; + readonly kernelsChanged: Event; /** * Default kernel @@ -314,7 +314,7 @@ export interface INotebookModel { /** * Event fired on active cell change */ - readonly onActiveCellChanged: Event; + readonly onActiveCellChanged: Event; /** * Event fired on cell type change @@ -389,7 +389,7 @@ export interface INotebookModel { * Get the standardKernelWithProvider by name * @param name The kernel name */ - getStandardKernelFromName(name: string): IStandardKernelWithProvider; + getStandardKernelFromName(name: string): IStandardKernelWithProvider | undefined; /** Event fired once we get call back from ConfigureConnection method in sqlops extension */ readonly onValidConnectionSelected: Event; diff --git a/src/sql/workbench/services/notebook/browser/models/notebookModel.ts b/src/sql/workbench/services/notebook/browser/models/notebookModel.ts index 1f1d713840..c20a52a1d0 100644 --- a/src/sql/workbench/services/notebook/browser/models/notebookModel.ts +++ b/src/sql/workbench/services/notebook/browser/models/notebookModel.ts @@ -49,43 +49,43 @@ export class NotebookModel extends Disposable implements INotebookModel { private _contextsChangedEmitter = new Emitter(); private _contextsLoadingEmitter = new Emitter(); private _contentChangedEmitter = new Emitter(); - private _kernelsChangedEmitter = new Emitter(); + private _kernelsChangedEmitter = new Emitter(); private _kernelChangedEmitter = new Emitter(); private _layoutChanged = new Emitter(); private _inErrorState: boolean = false; - private _activeClientSession: IClientSession; + private _activeClientSession: IClientSession | undefined; private _sessionLoadFinished = new Deferred(); private _onClientSessionReady = new Emitter(); private _onProviderIdChanged = new Emitter(); private _trustedMode: boolean; - private _onActiveCellChanged = new Emitter(); + private _onActiveCellChanged = new Emitter(); private _onCellTypeChanged = new Emitter(); - private _cells: ICellModel[]; - private _defaultLanguageInfo: nb.ILanguageInfo; - private _tags: string[]; - private _existingMetadata = {}; - private _language: string; + private _cells: ICellModel[] | undefined; + private _defaultLanguageInfo: nb.ILanguageInfo | undefined; + private _tags: string[] | undefined; + private _existingMetadata: nb.INotebookMetadata = {}; + private _language: string = ''; private _onErrorEmitter = new Emitter(); - private _savedKernelInfo: nb.IKernelInfo; + private _savedKernelInfo: nb.IKernelSpec | undefined; private readonly _nbformat: number = nbversion.MAJOR_VERSION; private readonly _nbformatMinor: number = nbversion.MINOR_VERSION; - private _activeConnection: ConnectionProfile; - private _activeCell: ICellModel; + private _activeConnection: ConnectionProfile | undefined; + private _activeCell: ICellModel | undefined; private _providerId: string; private _defaultKernel: nb.IKernelSpec; private _kernelDisplayNameToConnectionProviderIds: Map = new Map(); private _kernelDisplayNameToNotebookProviderIds: Map = new Map(); private _onValidConnectionSelected = new Emitter(); - private _oldKernel: nb.IKernel; + private _oldKernel: nb.IKernel | undefined; private _connectionUrisToDispose: string[] = []; private _textCellsLoading: number = 0; - private _standardKernels: notebookUtils.IStandardKernelWithProvider[]; + private _standardKernels: notebookUtils.IStandardKernelWithProvider[] = []; private _kernelAliases: string[] = []; - private _currentKernelAlias: string; - private _selectedKernelDisplayName: string; + private _currentKernelAlias: string | undefined; + private _selectedKernelDisplayName: string | undefined; - public requestConnectionHandler: () => Promise; + public requestConnectionHandler: (() => Promise) | undefined; constructor( private _notebookOptions: INotebookModelOptions, @@ -117,7 +117,7 @@ export class NotebookModel extends Disposable implements INotebookModel { return notebookManagers; } - public get notebookManager(): INotebookManager { + public get notebookManager(): INotebookManager | undefined { let manager = this.notebookManagers.find(manager => manager.providerId === this._providerId); if (!manager) { // Note: this seems like a less than ideal scenario. We should ideally pass in the "correct" provider ID and allow there to be a default, @@ -127,7 +127,7 @@ export class NotebookModel extends Disposable implements INotebookModel { return manager; } - public getNotebookManager(providerId: string): INotebookManager { + public getNotebookManager(providerId: string): INotebookManager | undefined { if (providerId) { return this.notebookManagers.find(manager => manager.providerId === providerId); } @@ -147,7 +147,7 @@ export class NotebookModel extends Disposable implements INotebookModel { public get hasServerManager(): boolean { // If the service has a server manager, then we can show the start button - return !!this.notebookManager.serverManager; + return !!this.notebookManager?.serverManager; } public get contentChanged(): Event { @@ -163,7 +163,7 @@ export class NotebookModel extends Disposable implements INotebookModel { * plus startup of the session manager which can return key metadata about the * notebook environment */ - public get clientSession(): IClientSession { + public get clientSession(): IClientSession | undefined { return this._activeClientSession; } @@ -171,7 +171,7 @@ export class NotebookModel extends Disposable implements INotebookModel { return this._kernelChangedEmitter.event; } - public get kernelsChanged(): Event { + public get kernelsChanged(): Event { return this._kernelsChangedEmitter.event; } @@ -191,17 +191,17 @@ export class NotebookModel extends Disposable implements INotebookModel { return this._contextsLoadingEmitter.event; } - public get cells(): ICellModel[] { + public get cells(): ICellModel[] | undefined { return this._cells; } - public get context(): ConnectionProfile { + public get context(): ConnectionProfile | undefined { return this._activeConnection; } public get specs(): nb.IAllKernels | undefined { let specs: nb.IAllKernels = { - defaultKernel: undefined, + defaultKernel: '', kernels: [] }; this.notebookManagers.forEach(manager => { @@ -241,11 +241,11 @@ export class NotebookModel extends Disposable implements INotebookModel { return this._kernelAliases; } - public get currentKernelAlias(): string { + public get currentKernelAlias(): string | undefined { return this._currentKernelAlias; } - public get selectedKernelDisplayName(): string { + public get selectedKernelDisplayName(): string | undefined { return this._selectedKernelDisplayName; } @@ -296,7 +296,7 @@ export class NotebookModel extends Disposable implements INotebookModel { return this._onValidConnectionSelected.event; } - public get onActiveCellChanged(): Event { + public get onActiveCellChanged(): Event { return this._onActiveCellChanged.event; } @@ -308,13 +308,13 @@ export class NotebookModel extends Disposable implements INotebookModel { return this._standardKernels; } - public set standardKernels(kernels) { + public set standardKernels(kernels: notebookUtils.IStandardKernelWithProvider[]) { this._standardKernels = kernels; this.setKernelDisplayNameMapsWithStandardKernels(); } public getApplicableConnectionProviderIds(kernelDisplayName: string): string[] { - let ids = []; + let ids; if (kernelDisplayName) { ids = this._kernelDisplayNameToConnectionProviderIds.get(kernelDisplayName); } @@ -325,7 +325,7 @@ export class NotebookModel extends Disposable implements INotebookModel { try { this._trustedMode = isTrusted; - let contents = null; + let contents: nb.INotebookContents | undefined; if (this._notebookOptions && this._notebookOptions.contentManager) { contents = await this._notebookOptions.contentManager.loadContent(); @@ -334,16 +334,17 @@ export class NotebookModel extends Disposable implements INotebookModel { // if cells already exist, create them with language info (if it is saved) this._cells = []; if (contents) { - this._defaultLanguageInfo = contents.metadata && contents.metadata.language_info; + this._defaultLanguageInfo = contents.metadata?.language_info; this._savedKernelInfo = this.getSavedKernelInfo(contents); if (contents.metadata) { //Telemetry of loading notebook - if (contents.metadata.azdata_notebook_guid && contents.metadata.azdata_notebook_guid.length === 36) { + let metadata: any = contents.metadata; + if (metadata.azdata_notebook_guid && metadata.azdata_notebook_guid.length === 36) { //Verify if it is actual GUID and then send it to the telemetry let regex = new RegExp('(\{){0,1}[0-9a-fA-F]{8}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{12}(\}){0,1}'); - if (regex.test(contents.metadata.azdata_notebook_guid)) { + if (regex.test(metadata.azdata_notebook_guid)) { this.adstelemetryService.createActionEvent(TelemetryKeys.TelemetryView.Notebook, TelemetryKeys.TelemetryAction.Open) - .withAdditionalProperties({ azdata_notebook_guid: contents.metadata.azdata_notebook_guid }) + .withAdditionalProperties({ azdata_notebook_guid: metadata.azdata_notebook_guid }) .send(); } } @@ -399,9 +400,9 @@ export class NotebookModel extends Disposable implements INotebookModel { return this._cells.findIndex(cell => cell.equals(cellModel)); } - public addCell(cellType: CellType, index?: number): ICellModel { + public addCell(cellType: CellType, index?: number): ICellModel | undefined { if (this.inErrorState) { - return null; + return undefined; } let cell = this.createCell(cellType); @@ -425,7 +426,7 @@ export class NotebookModel extends Disposable implements INotebookModel { moveCell(cell: ICellModel, direction: MoveDirection): void { if (this.inErrorState) { - return null; + return; } let index = this.findCellIndex(cell); @@ -457,12 +458,12 @@ export class NotebookModel extends Disposable implements INotebookModel { }); } - public updateActiveCell(cell: ICellModel): void { + public updateActiveCell(cell?: ICellModel): void { if (this._activeCell) { this._activeCell.active = false; } this._activeCell = cell; - if (cell) { + if (this._activeCell) { this._activeCell.active = true; } this._onActiveCellChanged.fire(cell); @@ -501,7 +502,7 @@ export class NotebookModel extends Disposable implements INotebookModel { if (index > -1) { this._cells.splice(index, 1); if (this._activeCell === cellModel) { - this.updateActiveCell(undefined); + this.updateActiveCell(); } this._contentChangedEmitter.fire({ changeType: NotebookChangeType.CellsModified, @@ -537,7 +538,7 @@ export class NotebookModel extends Disposable implements INotebookModel { } } - public get activeCell(): ICellModel { + public get activeCell(): ICellModel | undefined { return this._activeCell; } @@ -546,7 +547,7 @@ export class NotebookModel extends Disposable implements INotebookModel { } public async startSession(manager: INotebookManager, displayName?: string, setErrorStateOnFail?: boolean, kernelAlias?: string): Promise { - if (displayName && this._standardKernels) { + if (displayName) { let standardKernel = this._standardKernels.find(kernel => kernel.displayName === displayName); if (standardKernel) { this._defaultKernel = { name: standardKernel.name, display_name: standardKernel.displayName }; @@ -641,7 +642,7 @@ export class NotebookModel extends Disposable implements INotebookModel { this._defaultKernel = notebookConstants.sqlKernelSpec; this._providerId = SQL_NOTEBOOK_PROVIDER; } - if (!this._defaultLanguageInfo || this._defaultLanguageInfo.name) { + if (!this._defaultLanguageInfo?.name) { // update default language this._defaultLanguageInfo = { name: this._providerId === SQL_NOTEBOOK_PROVIDER ? 'sql' : 'python', @@ -651,46 +652,43 @@ export class NotebookModel extends Disposable implements INotebookModel { } private isValidConnection(profile: IConnectionProfile | connection.Connection) { - if (this._standardKernels) { - let standardKernels = this._standardKernels.find(kernel => this._defaultKernel && kernel.displayName === this._defaultKernel.display_name); - let connectionProviderIds = standardKernels ? standardKernels.connectionProviderIds : undefined; - let providerFeatures = this._capabilitiesService.getCapabilities(profile.providerName); - if (connectionProviderIds?.length) { - this._currentKernelAlias = providerFeatures?.connection.notebookKernelAlias; - // Switching from Kusto to another kernel should set the currentKernelAlias to undefined - if (this._selectedKernelDisplayName !== this._currentKernelAlias && this._selectedKernelDisplayName) { - this._currentKernelAlias = undefined; - } else { - // Adds Kernel Alias and Connection Provider to Map if new Notebook connection contains notebookKernelAlias - this._kernelDisplayNameToConnectionProviderIds.set(this._currentKernelAlias, [profile.providerName]); - } + let standardKernels = this._standardKernels.find(kernel => this._defaultKernel && kernel.displayName === this._defaultKernel.display_name); + let connectionProviderIds = standardKernels ? standardKernels.connectionProviderIds : undefined; + let providerFeatures = this._capabilitiesService?.getCapabilities(profile.providerName); + if (connectionProviderIds?.length) { + this._currentKernelAlias = providerFeatures?.connection.notebookKernelAlias; + // Switching from Kusto to another kernel should set the currentKernelAlias to undefined + if (this._selectedKernelDisplayName !== this._currentKernelAlias && this._selectedKernelDisplayName) { + this._currentKernelAlias = undefined; + } else { + // Adds Kernel Alias and Connection Provider to Map if new Notebook connection contains notebookKernelAlias + this._kernelDisplayNameToConnectionProviderIds.set(this._currentKernelAlias, [profile.providerName]); } - return this._currentKernelAlias || profile && connectionProviderIds && connectionProviderIds.find(provider => provider === profile.providerName) !== undefined; } - return false; + return this._currentKernelAlias || profile && connectionProviderIds && connectionProviderIds.find(provider => provider === profile.providerName) !== undefined; } - public getStandardKernelFromName(name: string): notebookUtils.IStandardKernelWithProvider { - if (name && this._standardKernels) { + public getStandardKernelFromName(name: string): notebookUtils.IStandardKernelWithProvider | undefined { + if (name) { let kernel = this._standardKernels.find(kernel => kernel.name.toLowerCase() === name.toLowerCase()); return kernel; } return undefined; } - public getStandardKernelFromDisplayName(displayName: string): notebookUtils.IStandardKernelWithProvider { - if (displayName && this._standardKernels) { + public getStandardKernelFromDisplayName(displayName: string): notebookUtils.IStandardKernelWithProvider | undefined { + if (displayName) { let kernel = this._standardKernels.find(kernel => kernel.displayName.toLowerCase() === displayName.toLowerCase()); return kernel; } return undefined; } - public get tags(): string[] { + public get tags(): string[] | undefined { return this._tags; } - public get languageInfo(): nb.ILanguageInfo { + public get languageInfo(): nb.ILanguageInfo | undefined { return this._defaultLanguageInfo; } @@ -710,7 +708,7 @@ export class NotebookModel extends Disposable implements INotebookModel { // If the "name" property isn't defined, check the "mimeType" property // Otherwise, default to python as the language let languageInfo = this.languageInfo; - let language: string; + let language: string = ''; if (languageInfo) { if (languageInfo.codemirror_mode) { let codeMirrorMode: nb.ICodeMirrorMode = (languageInfo.codemirror_mode); @@ -736,6 +734,8 @@ export class NotebookModel extends Disposable implements INotebookModel { } else if (language.toLowerCase() === 'c#') { language = 'cs'; } + } else { + language = 'python'; } this._language = language.toLowerCase(); @@ -743,8 +743,8 @@ export class NotebookModel extends Disposable implements INotebookModel { public changeKernel(displayName: string): void { this._selectedKernelDisplayName = displayName; - this._currentKernelAlias = this.context?.serverCapabilities.notebookKernelAlias; - if (this.kernelAliases.includes(this.currentKernelAlias) && displayName === this.currentKernelAlias) { + this._currentKernelAlias = this.context?.serverCapabilities?.notebookKernelAlias; + if (this._currentKernelAlias && this.kernelAliases.includes(this._currentKernelAlias) && displayName === this._currentKernelAlias) { this.doChangeKernel(displayName, true).catch(e => this.logService.error(e)); } else { this._currentKernelAlias = undefined; @@ -759,7 +759,7 @@ export class NotebookModel extends Disposable implements INotebookModel { return; } let oldDisplayName = this._activeClientSession && this._activeClientSession.kernel ? this._activeClientSession.kernel.name : undefined; - let nbKernelAlias: string; + let nbKernelAlias: string | undefined; if (this.kernelAliases.includes(displayName)) { this._currentKernelAlias = displayName; displayName = 'SQL'; @@ -778,7 +778,7 @@ export class NotebookModel extends Disposable implements INotebookModel { if (this._activeClientSession && this._activeClientSession.isReady) { let kernel = await this._activeClientSession.changeKernel(spec, this._oldKernel); try { - await kernel.ready; + await kernel?.ready; await this.updateKernelInfoOnKernelChange(kernel, nbKernelAlias); } catch (err2) { // TODO should we handle this in any way? @@ -827,7 +827,7 @@ export class NotebookModel extends Disposable implements INotebookModel { // Ensure that the kernel we try to switch to is a valid kernel; if not, use the default let kernelSpecs = this.getKernelSpecs(); 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); + spec = kernelSpecs.find(spec => spec.name === this.notebookManager?.sessionManager.specs.defaultKernel); } } else { @@ -874,6 +874,7 @@ export class NotebookModel extends Disposable implements INotebookModel { private setActiveConnectionIfDifferent(newConnection: ConnectionProfile) { if (this.isValidConnection(newConnection) && + this._activeConnection && this._activeConnection.id !== '-1' && this._activeConnection.id !== newConnection.id) { // Change the active connection to newConnection @@ -883,11 +884,11 @@ export class NotebookModel extends Disposable implements INotebookModel { // Get default kernel info if saved in notebook file - private getSavedKernelInfo(notebook: nb.INotebookContents): nb.IKernelInfo { + private getSavedKernelInfo(notebook: nb.INotebookContents): nb.IKernelSpec | undefined { return (notebook && notebook.metadata && notebook.metadata.kernelspec) ? notebook.metadata.kernelspec : undefined; } - private getKernelSpecFromDisplayName(displayName: string): nb.IKernelSpec { + private getKernelSpecFromDisplayName(displayName: string): nb.IKernelSpec | undefined { let kernel: nb.IKernelSpec = this.specs.kernels.find(k => k.display_name.toLowerCase() === displayName.toLowerCase()); if (!kernel) { return undefined; // undefined is handled gracefully in the session to default to the default kernel @@ -897,25 +898,23 @@ export class NotebookModel extends Disposable implements INotebookModel { return kernel; } - private sanitizeSavedKernelInfo() { + private sanitizeSavedKernelInfo(): void { if (this._savedKernelInfo) { let displayName = this._savedKernelInfo.display_name; if (this._savedKernelInfo.display_name !== displayName) { this._savedKernelInfo.display_name = displayName; } - if (this._standardKernels) { - let standardKernel = this._standardKernels.find(kernel => kernel.displayName === displayName || startsWith(displayName, kernel.displayName)); - if (standardKernel && this._savedKernelInfo.name && this._savedKernelInfo.name !== standardKernel.name) { - this._savedKernelInfo.name = standardKernel.name; - this._savedKernelInfo.display_name = standardKernel.displayName; - } + let standardKernel = this._standardKernels.find(kernel => kernel.displayName === displayName || startsWith(displayName, kernel.displayName)); + if (standardKernel && this._savedKernelInfo.name && this._savedKernelInfo.name !== standardKernel.name) { + this._savedKernelInfo.name = standardKernel.name; + this._savedKernelInfo.display_name = standardKernel.displayName; } } } - public getDisplayNameFromSpecName(kernel: nb.IKernel): string { - let specs = this.notebookManager.sessionManager.specs; + public getDisplayNameFromSpecName(kernel: nb.IKernel): string | undefined { + let specs = this.notebookManager?.sessionManager.specs; if (!specs || !specs.kernels) { return kernel.name; } @@ -972,7 +971,10 @@ export class NotebookModel extends Disposable implements INotebookModel { private async loadActiveContexts(kernelChangedArgs: nb.IKernelChangedArgs): Promise { if (kernelChangedArgs && kernelChangedArgs.newValue && kernelChangedArgs.newValue.name) { let kernelDisplayName = this.getDisplayNameFromSpecName(kernelChangedArgs.newValue); - let context = NotebookContexts.getContextForKernel(this._activeConnection, this.getApplicableConnectionProviderIds(kernelDisplayName)); + let context; + if (this._activeConnection) { + context = NotebookContexts.getContextForKernel(this._activeConnection, this.getApplicableConnectionProviderIds(kernelDisplayName)); + } if (context !== undefined && context.serverName !== undefined && context.title !== undefined) { await this.changeContext(context.title, context); } @@ -989,7 +991,7 @@ export class NotebookModel extends Disposable implements INotebookModel { display_name: spec.display_name, language: spec.language }; - this.clientSession.configureKernel(this._savedKernelInfo); + this.clientSession?.configureKernel(this._savedKernelInfo); } catch (err) { // Don't worry about this for now. Just use saved values } @@ -1024,7 +1026,7 @@ export class NotebookModel extends Disposable implements INotebookModel { return false; } - private tryFindProviderForKernel(displayName: string, alwaysReturnId: boolean = false): string { + private tryFindProviderForKernel(displayName: string, alwaysReturnId: boolean = false): string | undefined { if (!displayName) { return undefined; } @@ -1064,7 +1066,7 @@ export class NotebookModel extends Disposable implements INotebookModel { // Disconnect any connections that were added through the "Change connection" functionality in the Attach To dropdown private async disconnectAttachToConnections(): Promise { - notebookUtils.asyncForEach(this._connectionUrisToDispose, async conn => { + notebookUtils.asyncForEach(this._connectionUrisToDispose, async (conn: string) => { await this.notebookOptions.connectionService.disconnect(conn).catch(e => this.logService.error(e)); }); this._connectionUrisToDispose = []; @@ -1095,23 +1097,21 @@ export class NotebookModel extends Disposable implements INotebookModel { * provider and notebook provider ids from a kernel display name */ private setKernelDisplayNameMapsWithStandardKernels(): void { - if (this._standardKernels) { - this._standardKernels.forEach(kernel => { - let displayName = kernel.displayName; - if (!displayName) { - displayName = kernel.name; - } - this._kernelDisplayNameToConnectionProviderIds.set(displayName, kernel.connectionProviderIds); - this._kernelDisplayNameToNotebookProviderIds.set(displayName, kernel.notebookProvider); - }); - } + this._standardKernels.forEach(kernel => { + let displayName = kernel.displayName; + if (!displayName) { + displayName = kernel.name; + } + this._kernelDisplayNameToConnectionProviderIds.set(displayName, kernel.connectionProviderIds); + this._kernelDisplayNameToNotebookProviderIds.set(displayName, kernel.notebookProvider); + }); } /** * Serialize the model to JSON. */ toJSON(): nb.INotebookContents { - let cells: nb.ICellContents[] = this.cells.map(c => c.toJSON()); + let cells: nb.ICellContents[] = this.cells?.map(c => c.toJSON()); let metadata = Object.create(null) as nb.INotebookMetadata; // TODO update language and kernel when these change metadata.kernelspec = this._savedKernelInfo; @@ -1150,7 +1150,7 @@ export class NotebookModel extends Disposable implements INotebookModel { serializationStateChanged(changeType: NotebookChangeType, cell?: ICellModel): void { let changeInfo: NotebookContentChange = { changeType: changeType, - cells: [cell] + cells: cell ? [cell] : [] }; this._contentChangedEmitter.fire(changeInfo); diff --git a/src/sql/workbench/services/notebook/browser/sql/sqlNotebookManager.ts b/src/sql/workbench/services/notebook/browser/sql/sqlNotebookManager.ts index d930c9e188..54acab43aa 100644 --- a/src/sql/workbench/services/notebook/browser/sql/sqlNotebookManager.ts +++ b/src/sql/workbench/services/notebook/browser/sql/sqlNotebookManager.ts @@ -28,7 +28,7 @@ export class SqlNotebookManager implements nb.NotebookProvider { return this._contentManager; } - public get serverManager(): nb.ServerManager { + public get serverManager(): nb.ServerManager | undefined { return undefined; } diff --git a/src/sql/workbench/services/notebook/browser/sql/sqlSessionManager.ts b/src/sql/workbench/services/notebook/browser/sql/sqlSessionManager.ts index ea28d98ce9..286b95b2de 100644 --- a/src/sql/workbench/services/notebook/browser/sql/sqlSessionManager.ts +++ b/src/sql/workbench/services/notebook/browser/sql/sqlSessionManager.ts @@ -51,6 +51,26 @@ export interface SQLData { rows: Array>; } +export interface NotebookConfig { + cellToolbarLocation: string; + collapseBookItems: boolean; + diff: { enablePreview: boolean }; + displayOrder: Array; + kernelProviderAssociations: Array; + maxBookSearchDepth: number; + maxTableRows: number; + overrideEditorTheming: boolean; + pinnedNotebooks: Array; + pythonPath: string; + remoteBookDownloadTimeout: number; + showAllKernels: boolean; + showCellStatusBar: boolean; + showNotebookConvertActions: boolean; + sqlStopOnError: boolean; + trustedBooks: Array; + useExistingPython: boolean; +} + export class SqlSessionManager implements nb.SessionManager { private static _sessions: nb.ISession[] = []; @@ -162,8 +182,8 @@ class SqlKernel extends Disposable implements nb.IKernel { private _currentConnectionProfile: ConnectionProfile; static kernelId: number = 0; - private _id: string; - private _future: SQLFuture; + private _id: string | undefined; + private _future: SQLFuture | undefined; private _executionCount: number = 0; private _magicToExecutorMap = new Map(); private _connectionPath: string; @@ -301,7 +321,7 @@ class SqlKernel extends Disposable implements nb.IKernel { } // TODO should we cleanup old future? I don't think we need to - return this._future; + return this._future; } private getCodeWithoutCellMagic(content: nb.IExecuteRequest): string { @@ -377,9 +397,9 @@ class SqlKernel extends Disposable implements nb.IKernel { } export class SQLFuture extends Disposable implements FutureInternal { - private _msg: nb.IMessage = undefined; - private ioHandler: nb.MessageHandler; - private doneHandler: nb.MessageHandler; + private _msg: nb.IMessage | undefined; + private ioHandler: nb.MessageHandler | undefined; + private doneHandler: nb.MessageHandler | undefined; private doneDeferred = new Deferred(); private configuredMaxRows: number = MAX_ROWS; private _outputAddedPromises: Promise[] = []; @@ -387,13 +407,13 @@ export class SQLFuture extends Disposable implements FutureInternal { private _errorOccurred: boolean = false; private _stopOnError: boolean = true; constructor( - private _queryRunner: QueryRunner, + private _queryRunner: QueryRunner | undefined, private _executionCount: number | undefined, configurationService: IConfigurationService, private readonly logService: ILogService ) { super(); - let config = configurationService.getValue(NotebookConfigSectionName); + let config: NotebookConfig = configurationService.getValue(NotebookConfigSectionName); if (config) { let maxRows = config[MaxTableRowsConfigName] ? config[MaxTableRowsConfigName] : undefined; if (maxRows && maxRows > 0) { @@ -404,14 +424,16 @@ export class SQLFuture extends Disposable implements FutureInternal { } get inProgress(): boolean { - return this._queryRunner && !this._queryRunner.hasCompleted; + return this._queryRunner ? !this._queryRunner.hasCompleted : false; } + set inProgress(val: boolean) { if (this._queryRunner && !val) { this._queryRunner.cancelQuery().catch(err => onUnexpectedError(err)); } } - get msg(): nb.IMessage { + + get msg(): nb.IMessage | undefined { return this._msg; } @@ -468,7 +490,9 @@ export class SQLFuture extends Disposable implements FutureInternal { message = this.convertToDisplayMessage(msg); } } - this.ioHandler.handle(message); + if (message) { + this.ioHandler.handle(message); + } } } @@ -545,7 +569,7 @@ export class SQLFuture extends Disposable implements FutureInternal { metadata: undefined, parent_header: undefined }; - this.ioHandler.handle(msg); + this.ioHandler?.handle(msg); this._querySubsetResultMap.delete(resultSet.id); } @@ -605,7 +629,7 @@ export class SQLFuture extends Disposable implements FutureInternal { return htmlStringArr; } - private convertToDisplayMessage(msg: IResultMessage | string): nb.IIOPubMessage { + private convertToDisplayMessage(msg: IResultMessage | string): nb.IIOPubMessage | undefined { if (msg) { let msgData = typeof msg === 'string' ? msg : msg.message; return { @@ -627,7 +651,7 @@ export class SQLFuture extends Disposable implements FutureInternal { return undefined; } - private convertToError(msg: IResultMessage | string): nb.IIOPubMessage { + private convertToError(msg: IResultMessage | string): nb.IIOPubMessage | undefined { this._errorOccurred = true; if (msg) {