Merge pull request #2490 from rgbkrk/more-kernel-ops

create more selectors, apply generously
nteract · Feb 7, 2018 · ce305cb · ce305cb
2 parents 0326d73 + d7b24c4
commit ce305cb
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 58 deletions.
diff --git a/applications/desktop/src/notebook/epics/zeromq-kernels.js b/applications/desktop/src/notebook/epics/zeromq-kernels.js
@@ -43,6 +43,12 @@ import type { NewKernelAction } from "@nteract/core/actionTypes";
 
 import type { KernelInfo, LocalKernelProps } from "@nteract/types/core/records";
 
+import {
+  getCurrentKernel,
+  getCurrentHost,
+  isCurrentKernelZeroMQ
+} from "@nteract/core/selectors";
+
 import {
   createMessage,
   childOf,
@@ -179,8 +185,7 @@ export const launchKernelEpic = (
     // We must kill the previous kernel now
     // Then launch the next one
     switchMap(action => {
-      const state = store.getState();
-      const kernel = state.app.kernel;
+      const kernel = getCurrentKernel(store.getState());
 
       return merge(
         launchKernelObservable(action.kernelSpec, action.cwd),
@@ -196,25 +201,12 @@ export const launchKernelEpic = (
 export const interruptKernelEpic = (action$: *, store: *): Observable<Action> =>
   action$.pipe(
     ofType(INTERRUPT_KERNEL),
-    filter(() => {
-      const state = store.getState();
-      const host = state.app.host;
-      const kernel = state.app.kernel;
-
-      // This epic can only interrupt direct zeromq connected kernels
-      return (
-        host &&
-        kernel &&
-        host.type === "local" &&
-        kernel.type === "zeromq" &&
-        kernel.spawn
-      );
-    }),
+    // This epic can only interrupt direct zeromq connected kernels
+    filter(() => isCurrentKernelZeroMQ(store.getState())),
     // If the user fires off _more_ interrupts, we shouldn't interrupt the in-flight
     // interrupt, instead doing it after the last one happens
     concatMap(() => {
-      const state = store.getState();
-      const kernel = state.app.kernel;
+      const kernel = getCurrentKernel(store.getState());
 
       const spawn = kernel.spawn;
 
@@ -314,25 +306,10 @@ function killKernel(kernel): Observable<Action> {
 export const killKernelEpic = (action$: *, store: *): Observable<Action> =>
   action$.pipe(
     ofType(KILL_KERNEL),
-    filter(() => {
-      const state = store.getState();
-      const host = state.app.host;
-      const kernel = state.app.kernel;
-
-      // This epic can only interrupt direct zeromq connected kernels
-      return (
-        host &&
-        kernel &&
-        host.type === "local" &&
-        kernel.type === "zeromq" &&
-        kernel.spawn &&
-        kernel.channels
-      );
-    }),
+    // This epic can only interrupt direct zeromq connected kernels
+    filter(() => isCurrentKernelZeroMQ(store.getState())),
     concatMap(action => {
-      const app = store.getState().app;
-      const kernel = app.kernel;
-
+      const kernel = getCurrentKernel(store.getState());
       return killKernel(kernel);
     })
   );

diff --git a/packages/core/selectors.js b/packages/core/selectors.js
@@ -0,0 +1 @@
+module.exports = require("./").selectors;
diff --git a/packages/core/src/epics/execute.js b/packages/core/src/epics/execute.js
@@ -11,6 +11,8 @@ import {
   executionCounts
 } from "@nteract/messaging";
 
+import { getCurrentKernel } from "../selectors";
+
 import { Observable } from "rxjs/Observable";
 import { of } from "rxjs/observable/of";
 import { from } from "rxjs/observable/from";
@@ -135,9 +137,8 @@ export function createExecuteCellStream(
   message: ExecuteRequest,
   id: string
 ) {
-  const state = store.getState();
+  const kernel = getCurrentKernel(store.getState());
 
-  const kernel = state.app.kernel;
   const channels = kernel ? kernel.channels : null;
 
   const kernelConnected =

diff --git a/packages/core/src/epics/websocket-kernel.js b/packages/core/src/epics/websocket-kernel.js
@@ -25,7 +25,11 @@ import type { AppState, RemoteKernelProps } from "@nteract/types/core/records";
 import { kernels, shutdown } from "rx-jupyter";
 import { v4 as uuid } from "uuid";
 
-import { getServerConfig } from "../selectors";
+import {
+  getServerConfig,
+  isCurrentHostJupyter,
+  isCurrentKernelJupyterWebsocket
+} from "../selectors";
 
 import {
   LAUNCH_KERNEL,
@@ -40,10 +44,7 @@ export const launchWebSocketKernelEpic = (action$: *, store: *) =>
   action$.pipe(
     ofType(LAUNCH_KERNEL_BY_NAME),
     // Only accept jupyter servers for the host with this epic
-    filter(action => {
-      const host = store.getState().app.host;
-      return host && host.type === "jupyter" && host.serverUrl;
-    }),
+    filter(() => isCurrentHostJupyter(store.getState())),
     // TODO: When a switchMap happens, we need to close down the originating
     // kernel, likely by sending a different action. Right now this gets
     // coordinated in a different way.
@@ -71,21 +72,8 @@ export const launchWebSocketKernelEpic = (action$: *, store: *) =>
 export const interruptKernelEpic = (action$: *, store: *) =>
   action$.pipe(
     ofType(INTERRUPT_KERNEL),
-    filter(() => {
-      const state = store.getState();
-      const host = state.app.host;
-      const kernel = state.app.kernel;
-
-      // This epic can only interrupt kernels on jupyter websockets
-      return (
-        host &&
-        kernel &&
-        host.type === "jupyter" &&
-        host.serverUrl &&
-        kernel.id &&
-        kernel.type === "websocket"
-      );
-    }),
+    // This epic can only interrupt kernels on jupyter websockets
+    filter(() => isCurrentKernelJupyterWebsocket(store.getState())),
     // If the user fires off _more_ interrupts, we shouldn't interrupt the in-flight
     // interrupt, instead doing it after the last one happens
     concatMap(() => {

diff --git a/packages/core/src/index.js b/packages/core/src/index.js
@@ -7,6 +7,7 @@ import * as components from "./components";
 import * as providers from "./providers";
 import * as themes from "./themes";
 import * as epics from "./epics";
+import * as selectors from "./selectors";
 
 // keeping with backwards compatiblity for now
 const constants = actionTypes;
@@ -20,5 +21,6 @@ export {
   components,
   providers,
   themes,
+  selectors,
   epics
 };
diff --git a/packages/core/src/selectors/index.js b/packages/core/src/selectors/index.js
@@ -15,3 +15,53 @@ export const getServerConfig = createSelector(
     token
   })
 );
+
+// Quick memoized host and kernel selection.
+//
+// Intended to be useful for a core app and be future proof for when we have
+// both refs and selected/current hosts and kernels
+export const getCurrentHost = createSelector(
+  (state: AppState) => state.app.host,
+  host => host
+);
+
+export const getCurrentKernel = createSelector(
+  (state: AppState) => state.app.kernel,
+  kernel => kernel
+);
+
+export const getCurrentKernelType = createSelector(
+  [getCurrentKernel],
+  kernel => {
+    if (kernel && kernel.type) {
+      return kernel.type;
+    }
+    return null;
+  }
+);
+
+export const getCurrentHostType = createSelector([getCurrentHost], host => {
+  if (host && host.type) {
+    return host.type;
+  }
+  return null;
+});
+
+export const isCurrentKernelZeroMQ = createSelector(
+  [getCurrentHostType, getCurrentKernelType],
+  (hostType, kernelType) => {
+    return hostType === "local" && kernelType === "zeromq";
+  }
+);
+
+export const isCurrentHostJupyter = createSelector(
+  [getCurrentHostType],
+  hostType => hostType === "jupyter"
+);
+
+export const isCurrentKernelJupyterWebsocket = createSelector(
+  [getCurrentHostType, getCurrentKernelType],
+  (hostType, kernelType) => {
+    return hostType === "jupyter" && kernelType === "websocket";
+  }
+);