From 9205805935c33c46241f369cbc8e4eaf5b3f3d94 Mon Sep 17 00:00:00 2001 From: Nathaniel Nicandro Date: Mon, 6 Apr 2020 00:16:08 -0500 Subject: [PATCH] Refactor `jupyter-handle-message` and related Reduce the number of arguments `jupyter-handle-message` takes, by just passing the message property list directly. Expanding out the arguments just creates unnecessary work. Make the `jupyter-kernel-client' implementation of `jupyter-handle-message` more readable. Remove the unnecessary and confusing handler method dispatching. Document `jupyter-handle-message`. * README.org: Update reference to `jupyter-handle-*` method. * jupyter-client.el (jupyter-dispatch-messages-cases): Remove. (jupyter-run-hook-with-args-until-success): Remove. (jupyter-handle-message-p): New function. Similar to the above except its return value is negated. (jupyter--client-handlers): New variable. Holds the table of message handlers. (jupyter--handler-dispatch): Use them. (jupyter--run-callbacks): Change to `defsubst` (jupyter--run-handler-maybe): Remove. Expand out at call sites, replace `jupyter-handle-message` with `jupyter--handler-dispatch` at the sites. (jupyter--handler-dispatch): New function. Unifies all the `jupyter-handle-message` implementations for channels. (jupyter--update-execution-state, jupyter--message-completes-request-p): New functions. (jupyter-handle-message): Remove channel implementations. Use the new functions in the client implementation. --- jupyter-client.el | 282 ++++++++++++++++++---------------------------- 1 file changed, 112 insertions(+), 170 deletions(-) diff --git a/jupyter-client.el b/jupyter-client.el index 1576af5..e556132 100644 --- a/jupyter-client.el +++ b/jupyter-client.el @@ -390,15 +390,6 @@ this is called." (jupyter-with-client-buffer client (add-hook hook function append t))) -(defun jupyter-run-hook-with-args-until-success (client hook &rest args) - "Run CLIENT's value for HOOK with the arguments ARGS. -CLIENT is passed as the first argument and then ARGS." - (jupyter-with-client-buffer client - (when jupyter--debug - (message "RUN-HOOK: %s" hook)) - (with-demoted-errors "Error in Jupyter message hook: %S" - (apply #'run-hook-with-args-until-success hook client args)))) - (defun jupyter-remove-hook (client hook function) "Remove from CLIENT's value of HOOK the function FUNCTION." (jupyter-with-client-buffer client @@ -554,7 +545,7 @@ back." ;;; Message callbacks -(defun jupyter--run-callbacks (req msg) +(defsubst jupyter--run-callbacks (req msg) "Run REQ's MSG callbacks. See `jupyter-add-callback'." (when-let (callbacks (and req (jupyter-request-callbacks req))) @@ -720,142 +711,134 @@ received for it and it is not the most recently sent request." (not (or (eq ihandlers t) (if (eq (car ihandlers) 'not) (not type) type))))) -(defun jupyter--run-handler-maybe (client channel req msg) - "Possibly run CLIENT's CHANNEL handler on REQ's received MSG." - (when (jupyter--run-handler-p req msg) - (jupyter-handle-message channel client req msg))) +(defun jupyter-handle-message-p (client channel msg) + "Return non-nil if CLIENT should handle a MSG received on CHANNEL. +Run CLIENT's CHANNEL hook, jupyter-CHANNEL-message-hook, +passing (CLIENT MSG) as arguments to the hook functions. If all +of the hook functions return nil, then MSG should be handled. +nil is returned otherwise." + (jupyter-with-client-buffer client + (let ((hook (pcase channel + (:iopub 'jupyter-iopub-message-hook) + (:shell 'jupyter-shell-message-hook) + (:stdin 'jupyter-stdin-message-hook) + (_ (error "Unhandled channel: %s" channel))))) + (when jupyter--debug + (message "RUN-HOOK: %s" hook)) + (with-demoted-errors "Error in Jupyter message hook: %S" + (not (run-hook-with-args-until-success + hook client msg)))))) + +(defconst jupyter--client-handlers + (cl-labels + ((handler-alist + (&rest msg-types) + (cl-loop + for mt in msg-types + collect (cons mt (intern + (format "jupyter-handle-%s" + (substring (symbol-name mt) 1))))))) + `((:iopub . ,(handler-alist + :shutdown-reply :stream :comm-open :comm-msg + :comm-close :execute-input :execute-result + :error :status :clear-output :display-data + :update-display-data)) + (:shell . ,(handler-alist + :execute-reply :shutdown-reply :inspect-reply + :complete-reply :history-reply :is-complete-reply + :comm-info-reply :kernel-info-reply)) + (:stdin . ,(handler-alist + :input-reply :input-request))))) + +(defun jupyter--handler-dispatch (client channel msg req) + (when (jupyter-handle-message-p client channel msg) + (let* ((msg-type (jupyter-message-type msg)) + (channel-handlers + (or (alist-get channel jupyter--client-handlers) + (error "Unhandled channel: %s" channel))) + (handler (or (alist-get msg-type channel-handlers) + (error "Unhandled message type: %s" msg-type)))) + (funcall handler client req msg)))) + +(defsubst jupyter--update-execution-state (client msg req) + (pcase (jupyter-message-type msg) + (:status + (oset client execution-state + (jupyter-message-get msg :execution_state))) + ((or :execute-input + (and (guard req) :execute-reply)) + (oset client execution-count + (1+ (jupyter-message-get msg :execution_count)))))) + +(defsubst jupyter--message-completes-request-p (msg) + (or (jupyter-message-status-idle-p msg) + ;; Jupyter protocol 5.1, IPython implementation 7.5.0 + ;; doesn't give status: busy or status: idle messages on + ;; kernel-info-requests. Whereas IPython implementation + ;; 6.5.0 does. Seen on Appveyor tests. + ;; + ;; TODO: May be related jupyter/notebook#3705 as the + ;; problem does happen after a kernel restart when + ;; testing. + (eq (jupyter-message-type msg) :kernel-info-reply) + ;; No idle message is received after a shutdown reply so + ;; consider REQ as having received an idle message in + ;; this case. + (eq (jupyter-message-type msg) :shutdown-reply))) + +(cl-defgeneric jupyter-handle-message ((client jupyter-kernel-client) channel msg) + "Process a message received on CLIENT's CHANNEL. +CHANNEL is the Jupyter channel that MSG was received on by +CLIENT. MSG is a message property list and is the Jupyter +message being handled.") (cl-defmethod jupyter-handle-message ((client jupyter-kernel-client) channel msg) - "Process a message on CLIENT's CHANNEL. -When a message is received from the kernel, the -`jupyter-handle-message' method is called on the client. The -client method runs any callbacks for the message and possibly -runs the client handler for the channel the message was received -on. The channel's `jupyter-handle-message' method will then pass -the message to the appropriate message handler based on the -message type. + "Run callbacks and handler method for MSG. +Before any handling of MSG takes place, update CLIENT's execution +status slots (execution-state, execution-count) based on MSG, let +bind `jupyter-current-client' to CLIENT, and, when there is a +`jupyter-request' sent by CLIENT associated with the +`jupyter-message-parent-id' of MSG, set the +`jupyter-request-last-message' of the request to MSG. -So when a message is received from the kernel the following steps -are taken: +CLIENT may not have sent the request that generated MSG, e.g. if +MSG is an :execute-input request broadcasted to :iopub and not +sent by CLIENT. In this case, a message handler method is run, +without running any message callbacks, only if +`jupyter-include-other-output' is non-nil for CLIENT. -- `jupyter-handle-message' (client) - - Run callbacks for message - - Possibly run channel handlers - - `jupyter-handle-message' (channel) - - Based on message type, dispatch to - `jupyter-handle-execute-result', - `jupyter-handle-kernel-info-reply', ... - - Remove request from client request table when idle message is received" +When MSG has an associated request generated by CLIENT, run the +`jupyter-request-callbacks', if any, for the message before +attempting to run the message handler. Then remove old, +completed, requests from CLIENT's request table." (when msg - (let* ((jupyter-current-client client) - (pmsg-id (jupyter-message-parent-id msg)) - (requests (oref client requests)) - (req (gethash pmsg-id requests))) - ;; Update the state of the client - (pcase (jupyter-message-type msg) - (:status - (oset client execution-state - (jupyter-message-get msg :execution_state))) - ((or :execute-input (and (guard req) :execute-reply)) - (oset client execution-count - (1+ (jupyter-message-get msg :execution_count))))) - (if (not req) - (when (or (jupyter-get client 'jupyter-include-other-output) - ;; Always handle a startup message - (jupyter-message-status-starting-p msg)) - (jupyter--run-handler-maybe client channel req msg)) + (let ((jupyter-current-client client) + (req (gethash (jupyter-message-parent-id msg) (oref client requests)))) + (jupyter--update-execution-state client msg req) + (cond + (req (setf (jupyter-request-last-message req) msg) (unwind-protect (jupyter--run-callbacks req msg) (unwind-protect - (jupyter--run-handler-maybe client channel req msg) - (when (or (jupyter-message-status-idle-p msg) - ;; Jupyter protocol 5.1, IPython implementation 7.5.0 - ;; doesn't give status: busy or status: idle messages on - ;; kernel-info-requests. Whereas IPython implementation - ;; 6.5.0 does. Seen on Appveyor tests. - ;; - ;; TODO: May be related jupyter/notebook#3705 as the - ;; problem does happen after a kernel restart when - ;; testing. - (eq (jupyter-message-type msg) :kernel-info-reply) - ;; No idle message is received after a shutdown reply so - ;; consider REQ as having received an idle message in - ;; this case. - (eq (jupyter-message-type msg) :shutdown-reply)) + (when (jupyter--run-handler-p req msg) + (jupyter--handler-dispatch client channel msg req)) + (when (jupyter--message-completes-request-p msg) ;; Order matters here. We want to remove idle requests *before* ;; setting another request idle to account for idle messages ;; coming in out of order, e.g. before their respective reply ;; messages. (jupyter--drop-idle-requests client) - (setf (jupyter-request-idle-received-p req) t)))))))) - -;;; Channel handler macros - -(defmacro jupyter-dispatch-message-cases (client req msg cases) - "Dispatch to CLIENT handler's based on REQ and MSG. -CASES defines the handlers to dispatch to based on the -`jupyter-message-type' of MSG and should be a list of lists, the -first element of each inner list being the name of the handler, -excluding the `jupyter-handle-' prefix. The rest of the elements -in the list are the name of the keys that will be extracted from -the `jupyter-message-content' of MSG and passed to the handler in -the same order as they appear. For example, - - (jupyter-dispatch-message-cases client req msg - ((shutdown-reply restart) - (stream name text))) - -will be transformed to - - (let ((content (jupyter-message-content msg))) - (pcase (jupyter-message-type msg) - (:shutdown-reply - (cl-destructuring-bind (&key restart &allow-other-keys) - content - (jupyter-handle-shutdown-reply client req restart))) - (:stream - (cl-destructuring-bind (&key name text &allow-other-keys) - content - (jupyter-handle-stream client req name text))) - (_ (warn \"Message type not handled (%s)\" - (jupyter-message-type msg)))))" - (declare (indent 3)) - (let ((handlers nil) - (content (make-symbol "contentvar")) - (jclient (make-symbol "clientvar")) - (jreq (make-symbol "reqvar")) - (jmsg (make-symbol "msgvar"))) - (dolist (case cases) - (cl-destructuring-bind (msg-type . keys) case - (let ((handler (intern (format "jupyter-handle-%s" msg-type))) - (msg-type (intern (concat ":" (symbol-name msg-type))))) - (push `(,msg-type - (cl-destructuring-bind (&key ,@keys &allow-other-keys) - ,content - (,handler ,jclient ,jreq ,@keys))) - handlers)))) - `(let* ((,jmsg ,msg) - (,jreq ,req) - (,jclient ,client) - (,content (jupyter-message-content ,jmsg))) - (pcase (jupyter-message-type ,jmsg) - ,@handlers - (_ (warn "Message type not handled (%s)" - (jupyter-message-type msg))))))) + (setf (jupyter-request-idle-received-p req) t))))) + (t + (when (and (or (jupyter-get client 'jupyter-include-other-output) + ;; Always handle a startup message + (jupyter-message-status-starting-p msg)) + (jupyter--run-handler-p req msg)) + (jupyter--handler-dispatch client channel msg req))))))) ;;; STDIN handlers -(cl-defmethod jupyter-handle-message ((_channel (eql :stdin)) - client - req - msg) - (unless (jupyter-run-hook-with-args-until-success - client 'jupyter-stdin-message-hook msg) - (jupyter-dispatch-message-cases client req msg - ((input-reply prompt password) - (input-request prompt password))))) - (cl-defgeneric jupyter-handle-input-request ((client jupyter-kernel-client) _req prompt @@ -884,27 +867,6 @@ user. Otherwise `read-from-minibuffer' is used." (defalias 'jupyter-handle-input-reply 'jupyter-handle-input-request) -;;; SHELL handlers - -;; http://jupyter-client.readthedocs.io/en/latest/messaging.html#messages-on-the-shell-router-dealer-channel -(cl-defmethod jupyter-handle-message ((_channel (eql :shell)) - client - req - msg) - (unless (jupyter-run-hook-with-args-until-success - client 'jupyter-shell-message-hook msg) - (jupyter-dispatch-message-cases client req msg - ((execute-reply status execution_count user_expressions payload) - (shutdown-reply restart) - (inspect-reply found data metadata) - (complete-reply matches cursor_start cursor_end metadata) - (history-reply history) - (is-complete-reply status indent) - (comm-info-reply comms) - (kernel-info-reply protocol_version implementation - implementation_version language_info - banner help_links))))) - ;;;; Evaluation (cl-defgeneric jupyter-load-file-code (_file) @@ -2149,26 +2111,6 @@ If RESTART is non-nil, request a restart instead of a complete shutdown." ;;; IOPUB handlers -(cl-defmethod jupyter-handle-message ((_channel (eql :iopub)) - client - req - msg) - (unless (jupyter-run-hook-with-args-until-success - client 'jupyter-iopub-message-hook msg) - (jupyter-dispatch-message-cases client req msg - ((shutdown-reply restart) - (stream name text) - (comm-open comm_id target_name target_module data) - (comm-msg comm_id data) - (comm-close comm_id data) - (execute-input code execution_count) - (execute-result execution_count data metadata) - (error ename evalue traceback) - (status execution_state) - (clear-output wait) - (display-data data metadata transient) - (update-display-data data metadata transient))))) - (cl-defgeneric jupyter-handle-comm-open ((client jupyter-kernel-client) req id