From 92073a5646e71f39b69e49827e5e8e391bcee40e Mon Sep 17 00:00:00 2001 From: Laurent Nguyen Date: Fri, 28 Aug 2020 09:51:44 +0200 Subject: [PATCH] Start kernel when opening notebook tab (#168) * Add Auto-start kernel epic * Replace deprecated rxjs empty() with EMPTY constant * Fix format --- .../NotebookComponentBootstrapper.tsx | 2 +- .../Notebook/NotebookComponent/epics.test.ts | 127 +++++++++++++----- .../Notebook/NotebookComponent/epics.ts | 66 ++++++--- 3 files changed, 141 insertions(+), 54 deletions(-) diff --git a/src/Explorer/Notebook/NotebookComponent/NotebookComponentBootstrapper.tsx b/src/Explorer/Notebook/NotebookComponent/NotebookComponentBootstrapper.tsx index dfc64b161..f3933e8e4 100644 --- a/src/Explorer/Notebook/NotebookComponent/NotebookComponentBootstrapper.tsx +++ b/src/Explorer/Notebook/NotebookComponent/NotebookComponentBootstrapper.tsx @@ -98,7 +98,7 @@ export class NotebookComponentBootstrapper { actions.fetchContentFulfilled({ filepath: undefined, model: NotebookComponentBootstrapper.wrapModelIntoContent(name, undefined, content), - kernelRef: createKernelRef(), + kernelRef: undefined, // must be undefined or it will be auto-started by the epic contentRef: this.contentRef }) ); diff --git a/src/Explorer/Notebook/NotebookComponent/epics.test.ts b/src/Explorer/Notebook/NotebookComponent/epics.test.ts index ec6e95a47..5c95e563d 100644 --- a/src/Explorer/Notebook/NotebookComponent/epics.test.ts +++ b/src/Explorer/Notebook/NotebookComponent/epics.test.ts @@ -1,13 +1,13 @@ import * as Immutable from "immutable"; import { ActionsObservable, StateObservable } from "redux-observable"; -import { Subject } from "rxjs"; +import { Subject, empty } from "rxjs"; import { toArray } from "rxjs/operators"; import { makeNotebookRecord } from "@nteract/commutable"; import { actions, state } from "@nteract/core"; import * as sinon from "sinon"; import { CdbAppState, makeCdbRecord } from "./types"; -import { launchWebSocketKernelEpic } from "./epics"; +import { launchWebSocketKernelEpic, autoStartKernelEpic } from "./epics"; import { NotebookUtil } from "../NotebookUtil"; import { sessions } from "rx-jupyter"; @@ -74,46 +74,47 @@ describe("Extract kernel from notebook", () => { }); }); +const initialState = { + app: state.makeAppRecord({ + host: state.makeJupyterHostRecord({ + type: "jupyter", + token: "eh", + basePath: "/" + }) + }), + comms: state.makeCommsRecord(), + config: Immutable.Map({}), + core: state.makeStateRecord({ + kernelRef: "fake", + entities: state.makeEntitiesRecord({ + contents: state.makeContentsRecord({ + byRef: Immutable.Map({ + fakeContentRef: state.makeNotebookContentRecord() + }) + }), + kernels: state.makeKernelsRecord({ + byRef: Immutable.Map({ + fake: state.makeRemoteKernelRecord({ + type: "websocket", + channels: new Subject(), + kernelSpecName: "fancy", + id: "0" + }) + }) + }) + }) + }), + cdb: makeCdbRecord({ + databaseAccountName: "dbAccountName", + defaultExperience: "defaultExperience" + }) +}; + describe("launchWebSocketKernelEpic", () => { const createSpy = sinon.spy(sessions, "create"); const contentRef = "fakeContentRef"; const kernelRef = "fake"; - const initialState = { - app: state.makeAppRecord({ - host: state.makeJupyterHostRecord({ - type: "jupyter", - token: "eh", - basePath: "/" - }) - }), - comms: state.makeCommsRecord(), - config: Immutable.Map({}), - core: state.makeStateRecord({ - kernelRef: "fake", - entities: state.makeEntitiesRecord({ - contents: state.makeContentsRecord({ - byRef: Immutable.Map({ - fakeContentRef: state.makeNotebookContentRecord() - }) - }), - kernels: state.makeKernelsRecord({ - byRef: Immutable.Map({ - fake: state.makeRemoteKernelRecord({ - type: "websocket", - channels: new Subject(), - kernelSpecName: "fancy", - id: "0" - }) - }) - }) - }) - }), - cdb: makeCdbRecord({ - databaseAccountName: "dbAccountName", - defaultExperience: "defaultExperience" - }) - }; it("launches remote kernels", async () => { const state$ = new StateObservable(new Subject(), initialState); @@ -490,3 +491,55 @@ describe("launchWebSocketKernelEpic", () => { }); }); }); + +describe("autoStartKernelEpic", () => { + const contentRef = "fakeContentRef"; + const kernelRef = "fake"; + + it("automatically starts kernel when content fetch is successful if kernelRef is defined", async () => { + const state$ = new StateObservable(new Subject(), initialState); + + const action$ = ActionsObservable.of( + actions.fetchContentFulfilled({ + contentRef, + kernelRef, + filepath: "filepath", + model: {} + }) + ); + + const responseActions = await autoStartKernelEpic(action$, state$) + .pipe(toArray()) + .toPromise(); + + expect(responseActions).toMatchObject([ + { + type: actions.RESTART_KERNEL, + payload: { + contentRef, + kernelRef, + outputHandling: "None" + } + } + ]); + }); + + it("Don't start kernel when content fetch is successful if kernelRef is not defined", async () => { + const state$ = new StateObservable(new Subject(), initialState); + + const action$ = ActionsObservable.of( + actions.fetchContentFulfilled({ + contentRef, + kernelRef: undefined, + filepath: "filepath", + model: {} + }) + ); + + const responseActions = await autoStartKernelEpic(action$, state$) + .pipe(toArray()) + .toPromise(); + + expect(responseActions).toMatchObject([]); + }); +}); diff --git a/src/Explorer/Notebook/NotebookComponent/epics.ts b/src/Explorer/Notebook/NotebookComponent/epics.ts index 84b5a78dc..f65677591 100644 --- a/src/Explorer/Notebook/NotebookComponent/epics.ts +++ b/src/Explorer/Notebook/NotebookComponent/epics.ts @@ -1,4 +1,4 @@ -import { empty, merge, of, timer, concat, Subject, Subscriber, Observable, Observer } from "rxjs"; +import { EMPTY, merge, of, timer, concat, Subject, Subscriber, Observable, Observer } from "rxjs"; import { webSocket } from "rxjs/webSocket"; import { ActionsObservable, StateObservable } from "redux-observable"; import { ofType } from "redux-observable"; @@ -77,7 +77,7 @@ const addInitialCodeCellEpic = ( // If it's not a notebook, we shouldn't be here if (!model || model.type !== "notebook") { - return empty(); + return EMPTY; } const cellOrder = selectors.notebook.cellOrder(model); @@ -90,7 +90,40 @@ const addInitialCodeCellEpic = ( ); } - return empty(); + return EMPTY; + }) + ); +}; + +/** + * Automatically start kernel if kernelRef is present. + * The kernel is normally lazy-started when a cell is being executed, but a running kernel is + * required for code completion to work. + * For notebook viewer, there is no kernel + * @param action$ + * @param state$ + */ +export const autoStartKernelEpic = ( + action$: ActionsObservable, + state$: StateObservable +): Observable<{} | actions.CreateCellBelow> => { + return action$.pipe( + ofType(actions.FETCH_CONTENT_FULFILLED), + mergeMap(action => { + const state = state$.value; + const { contentRef, kernelRef } = action.payload; + + if (!kernelRef) { + return EMPTY; + } + + return of( + actions.restartKernel({ + contentRef, + kernelRef, + outputHandling: "None" + }) + ); }) ); }; @@ -288,7 +321,7 @@ export const launchWebSocketKernelEpic = ( const state = state$.value; const host = selectors.currentHost(state); if (host.type !== "jupyter") { - return empty(); + return EMPTY; } const serverConfig: NotebookServiceConfig = selectors.serverConfig(host); serverConfig.userPuid = getUserPuid(); @@ -299,7 +332,7 @@ export const launchWebSocketKernelEpic = ( const content = selectors.content(state, { contentRef }); if (!content || content.type !== "notebook") { - return empty(); + return EMPTY; } let kernelSpecToLaunch = kernelSpecName; @@ -513,26 +546,26 @@ const changeWebSocketKernelEpic = ( const state = state$.value; const host = selectors.currentHost(state); if (host.type !== "jupyter") { - return empty(); + return EMPTY; } const serverConfig: NotebookServiceConfig = selectors.serverConfig(host); if (!oldKernelRef) { - return empty(); + return EMPTY; } const oldKernel = selectors.kernel(state, { kernelRef: oldKernelRef }); if (!oldKernel || oldKernel.type !== "websocket") { - return empty(); + return EMPTY; } const { sessionId } = oldKernel; if (!sessionId) { - return empty(); + return EMPTY; } const content = selectors.content(state, { contentRef }); if (!content || content.type !== "notebook") { - return empty(); + return EMPTY; } const { filepath, @@ -593,7 +626,7 @@ const focusInitialCodeCellEpic = ( // If it's not a notebook, we shouldn't be here if (!model || model.type !== "notebook") { - return empty(); + return EMPTY; } const cellOrder = selectors.notebook.cellOrder(model); @@ -608,7 +641,7 @@ const focusInitialCodeCellEpic = ( ); } - return empty(); + return EMPTY; }) ); }; @@ -661,7 +694,7 @@ const notificationsToUserEpic = ( break; } } - return empty(); + return EMPTY; }) ); }; @@ -701,7 +734,7 @@ const handleKernelConnectionLostEpic = ( if (explorer) { explorer.showOkModalDialog("kernel restarts", msg); } - return of(empty()); + return of(EMPTY); } return concat( @@ -814,7 +847,7 @@ const closeUnsupportedMimetypesEpic = ( explorer.showOkModalDialog("File cannot be rendered", msg); NotificationConsoleUtils.logConsoleMessage(ConsoleDataType.Error, msg); } - return empty(); + return EMPTY; }) ); }; @@ -842,13 +875,14 @@ const closeContentFailedToFetchEpic = ( explorer.showOkModalDialog("Failure to load", msg); NotificationConsoleUtils.logConsoleMessage(ConsoleDataType.Error, msg); } - return empty(); + return EMPTY; }) ); }; export const allEpics = [ addInitialCodeCellEpic, + autoStartKernelEpic, focusInitialCodeCellEpic, notificationsToUserEpic, launchWebSocketKernelEpic,