Browse Source

Rework session and clientsession status to just point to the kernel status.

Jason Grout 5 years ago
parent
commit
924f4fe862

+ 3 - 18
packages/apputils/src/clientsession.tsx

@@ -85,11 +85,6 @@ export interface IClientSession extends IDisposable {
    */
   readonly type: string;
 
-  /**
-   * The current status of the client session.
-   */
-  readonly status: Kernel.Status;
-
   /**
    * Whether the session is ready.
    */
@@ -312,16 +307,6 @@ export class ClientSession implements IClientSession {
    */
   readonly manager: Session.IManager;
 
-  /**
-   * The current status of the session.
-   */
-  get status(): Kernel.Status {
-    if (!this.isReady) {
-      return 'starting';
-    }
-    return this._session ? this._session.status : 'dead';
-  }
-
   /**
    * Whether the session is ready.
    */
@@ -585,7 +570,7 @@ export class ClientSession implements IClientSession {
       return Promise.reject('Disposed');
     }
     let session = this._session;
-    if (session && session.status !== 'dead') {
+    if (session && session.kernel.status !== 'dead') {
       return session.changeKernel(options).catch(err => {
         void this._handleSessionError(err);
         return Promise.reject(err);
@@ -782,7 +767,7 @@ export class ClientSession implements IClientSession {
     // If we have already, and now we aren't busy, dispose
     // of the busy disposable.
     if (this._setBusy) {
-      if (this.status === 'busy') {
+      if (this.kernel.status === 'busy') {
         if (!this._busyDisposable) {
           this._busyDisposable = this._setBusy();
         }
@@ -794,7 +779,7 @@ export class ClientSession implements IClientSession {
       }
     }
 
-    this._statusChanged.emit(this.status);
+    this._statusChanged.emit(this.kernel.status);
   }
 
   /**

+ 1 - 1
packages/apputils/src/toolbar.tsx

@@ -704,7 +704,7 @@ namespace Private {
       if (this.isDisposed) {
         return;
       }
-      let status = session.status;
+      let status = session.kernel.status;
       const busy = this._isBusy(status);
       this.toggleClass(TOOLBAR_BUSY_CLASS, busy);
       this.toggleClass(TOOLBAR_IDLE_CLASS, !busy);

+ 7 - 4
packages/services/src/kernel/kernel.ts

@@ -997,14 +997,17 @@ export namespace Kernel {
    *
    * #### Notes
    * The status states are:
-   * * `unknown`: The kernel status is unkown, often because the connection is
-   *   disconnected or connecting. This state is determined by the kernel
+   * * `unknown`: The kernel status is unknown, often because the connection
+   *   is disconnected or connecting. This state is determined by the kernel
    *   connection status.
+   * * `autorestarting`: The kernel is restarting, initiated by the server.
+   *   This state is set by the services library, not explicitly sent from the
+   *   kernel.
    * * `starting`: The kernel is starting
    * * `idle`: The kernel has finished processing messages.
    * * `busy`: The kernel is currently processing messages.
-   * * `restarting`: The kernel is restarting. This state is sent by the Jupyter
-   *   server.
+   * * `restarting`: The kernel is restarting. This state is sent by the
+   *   Jupyter server.
    * * `dead`: The kernel is dead and will not be restarted. This state is set
    *   by the Jupyter server and is a final state.
    */

+ 41 - 5
packages/services/src/kernel/validate.ts

@@ -24,7 +24,12 @@ const IOPUB_CONTENT_FIELDS: { [key: string]: any } = {
     metadata: 'object'
   },
   error: { ename: 'string', evalue: 'string', traceback: 'object' },
-  status: { execution_state: 'string' },
+  status: {
+    execution_state: [
+      'string',
+      ['starting', 'idle', 'busy', 'restarting', 'dead']
+    ]
+  },
   clear_output: { wait: 'boolean' },
   comm_open: { comm_id: 'string', target_name: 'string', data: 'object' },
   comm_msg: { comm_id: 'string', data: 'object' },
@@ -34,15 +39,21 @@ const IOPUB_CONTENT_FIELDS: { [key: string]: any } = {
 
 /**
  * Validate a property as being on an object, and optionally
- * of a given type.
+ * of a given type and among a given set of values.
  */
-function validateProperty(object: any, name: string, typeName?: string): void {
+function validateProperty(
+  object: any,
+  name: string,
+  typeName?: string,
+  values: any[] = []
+): void {
   if (!object.hasOwnProperty(name)) {
     throw Error(`Missing property '${name}'`);
   }
+  const value = object[name];
+
   if (typeName !== void 0) {
     let valid = true;
-    let value = object[name];
     switch (typeName) {
       case 'array':
         valid = Array.isArray(value);
@@ -56,6 +67,27 @@ function validateProperty(object: any, name: string, typeName?: string): void {
     if (!valid) {
       throw new Error(`Property '${name}' is not of type '${typeName}`);
     }
+
+    if (values.length > 0) {
+      let valid = true;
+      switch (typeName) {
+        case 'string':
+        case 'number':
+        case 'boolean':
+          valid = values.includes(value);
+          break;
+        default:
+          valid = values.findIndex(v => v === value) >= 0;
+          break;
+      }
+      if (!valid) {
+        throw new Error(
+          `Property '${name}' is not one of the valid values ${JSON.stringify(
+            values
+          )}`
+        );
+      }
+    }
   }
 }
 
@@ -94,7 +126,11 @@ function validateIOPubContent(msg: KernelMessage.IIOPubMessage): void {
     let names = Object.keys(fields);
     let content = msg.content;
     for (let i = 0; i < names.length; i++) {
-      validateProperty(content, names[i], fields[names[i]]);
+      let args = fields[names[i]];
+      if (!Array.isArray(args)) {
+        args = [args];
+      }
+      validateProperty(content, names[i], ...args);
     }
   }
 }

+ 7 - 14
packages/services/src/session/default.ts

@@ -152,16 +152,6 @@ export class DefaultSession implements Session.ISession {
     };
   }
 
-  /**
-   * The current status of the session.
-   *
-   * #### Notes
-   * This is a delegate to the kernel status.
-   */
-  get status(): Kernel.Status {
-    return this._kernel ? this._kernel.status : 'dead';
-  }
-
   /**
    * The server settings of the session.
    */
@@ -224,7 +214,6 @@ export class DefaultSession implements Session.ISession {
     this._isDisposed = true;
     this._disposed.emit();
     this._kernel.dispose();
-    this._statusChanged.emit('dead');
     Private.removeRunning(this);
     Signal.clearData(this);
   }
@@ -293,7 +282,11 @@ export class DefaultSession implements Session.ISession {
     }
     let data = JSON.stringify({ kernel: options });
     this._kernel.dispose();
-    this._statusChanged.emit('restarting');
+
+    // This status is not technically correct, but it may be useful to refresh
+    // clients TODO: evaluate whether we want to do this, or tell people to
+    // listen to the kernelChanged signal.
+    // this._statusChanged.emit('restarting');
     return this._patch(data).then(() => this.kernel);
   }
 
@@ -810,8 +803,8 @@ namespace Private {
         }
         return false;
       });
-      // If session is no longer running on disk, emit dead signal.
-      if (!updated && session.status !== 'dead') {
+      // If session is no longer running on disk, dispose the session.
+      if (!updated && session.isDisposed !== true) {
         session.dispose();
       }
     });

+ 12 - 17
packages/services/src/session/session.ts

@@ -25,7 +25,9 @@ export namespace Session {
    * A session object represents a live connection to a session kernel.
    *
    * This represents a persistent kernel connection with a particular key,
-   * that persists across changing kernels and kernels getting terminated.
+   * that persists across changing kernels and kernels getting terminated. As
+   * such, a number of signals are proxied from the current kernel for
+   * convenience.
    */
   export interface ISession extends IObservableDisposable {
     /**
@@ -34,12 +36,13 @@ export namespace Session {
     kernelChanged: ISignal<this, IKernelChangedArgs>;
 
     /**
-     * A signal proxied from the current kernel about its status.
+     * The kernel statusChanged signal, proxied from the current kernel.
      */
     statusChanged: ISignal<this, Kernel.Status>;
 
     /**
-     * A signal proxied from the current kernel about its connection status.
+     * The kernel connectionStatusChanged signal, proxied from the current
+     * kernel.
      */
     connectionStatusChanged: ISignal<this, Kernel.ConnectionStatus>;
 
@@ -49,20 +52,17 @@ export namespace Session {
     readonly propertyChanged: ISignal<this, 'path' | 'name' | 'type'>;
 
     /**
-     * A signal emitted for iopub kernel messages.
+     * The kernel iopubMessage signal, proxied from the current kernel.
      */
     iopubMessage: ISignal<this, KernelMessage.IIOPubMessage>;
 
     /**
-     * A signal emitted for unhandled kernel message.
+     * The kernel unhandledMessage signal, proxied from the current kernel.
      */
     unhandledMessage: ISignal<this, KernelMessage.IMessage>;
 
     /**
-     * A signal emitted for any kernel message.
-     *
-     * Note: The behavior is undefined if the message is modified
-     * during message handling. As such, it should be treated as read-only.
+     * The kernel anyMessage signal, proxied from the current kernel.
      */
     anyMessage: ISignal<this, Kernel.IAnyMessageArgs>;
 
@@ -101,16 +101,11 @@ export namespace Session {
      *
      * #### Notes
      * This is a read-only property, and can be altered by [changeKernel].
-     */
-    readonly kernel: Kernel.IKernelConnection;
-
-    /**
-     * The current status of the session.
      *
-     * #### Notes
-     * This is a delegate to the kernel status.
+     * A number of kernel signals are proxied through the session from
+     * whatever the current kernel is for convenience.
      */
-    readonly status: Kernel.Status;
+    readonly kernel: Kernel.IKernelConnection;
 
     /**
      * Change the session path.

+ 1 - 1
packages/statusbar/src/defaults/kernelStatus.tsx

@@ -150,7 +150,7 @@ export namespace KernelStatus {
         this._kernelStatus = 'unknown';
         this._kernelName = 'unknown';
       } else {
-        this._kernelStatus = this._session.status;
+        this._kernelStatus = this._session.kernel.status;
         this._kernelName = this._session.kernelDisplayName;
 
         this._session.statusChanged.connect(this._onKernelStatusChanged);

+ 10 - 0
tests/test-services/src/kernel/validate.spec.ts

@@ -113,6 +113,16 @@ describe('kernel/validate', () => {
       expect(() => validateMessage(msg)).to.throw();
     });
 
+    it('should throw on invalid iopub status message content', () => {
+      const msg = KernelMessage.createMessage<KernelMessage.IStatusMsg>({
+        msgType: 'status',
+        channel: 'iopub',
+        session: 'baz',
+        content: { execution_state: 'invalid-status' as Kernel.Status }
+      });
+      expect(() => validateMessage(msg)).to.throw();
+    });
+
     it('should handle no buffers field', () => {
       const msg = KernelMessage.createMessage({
         msgType: 'comm_msg',

+ 1 - 1
tests/test-services/src/session/isession.spec.ts

@@ -192,7 +192,7 @@ describe('session', () => {
     });
 
     describe('#isDisposed', () => {
-      it('should be true after we dispose of the session', async () => {   
+      it('should be true after we dispose of the session', async () => {
         const session = await startNew();
         expect(session.isDisposed).to.equal(false);
         session.dispose();