search

jupyter / enhancement-proposals

Enhancement proposals for the Jupyter Ecosystem
https://jupyter.org/enhancement-proposals
BSD 3-Clause "New" or "Revised" License
115 stars 65 forks source link

JEP for Optional Features #92

Closed JohanMabille closed 1 year ago

JohanMabille commented 1 year ago

Following this discussion, here is the JEP for introducing optional features in the protocol.

Voting from @jupyter/software-steering-council

kevin-bates commented 1 year ago

Hi @JohanMabille - this is a good idea - thanks for writing it up.

I never know how detailed a JEP should be but have a couple of questions that may (or may not) be part of this JEP.

  1. Where will the canonical list of feature names be maintained? (I'm figuring in the messaging documentation in jupyter_client but this should be clarified). Seems like this area should also describe the feature and its expected behavior so kernel/feature authors know what is required.
  2. Should there be a corresponding version indicator associated with each feature (and whose syntax is specified in this proposal, e.g., semantic versioning, strictly increasing integer, etc.)?
JohanMabille commented 1 year ago

Where will the canonical list of feature names be maintained? (I'm figuring in the messaging documentation in jupyter_client but this should be clarified). Seems like this area should also describe the feature and its expected behavior so kernel/feature authors know what is required.

Indeed, like the other parts of the protocol, this list should be in the messaging documentation. I don't know if this has to be specified in the JEP though.

Should there be a corresponding version indicator associated with each feature (and whose syntax is specified in this proposal, e.g., semantic versioning, strictly increasing integer, etc.)?

I'm afraid that doing so can lead to unnecessary complexity when the number of optional features grows. I think it's ok to bump the major version of the protocol even if only optional features are backward incompatible.

minrk commented 1 year ago

Does it make sense for this to live (also?) at the kernelspec level? I'm thinking of #66, where handshaking may be considered a feature, rather than an unconditional version>= requirement. 👍 in general.

JohanMabille commented 1 year ago

The kernel handshake pattern itself should not be optional as discussed in https://github.com/jupyter/enhancement-proposals/pull/66. For other features, I think that as long as they do not concern the connection itself or data that a client should get before connecting to the kernel, having them in the kernel_info message is enough, no need to duplicate these information in the kernelspec.

I cannot think of an optional feature that would required to be in the kernelspec; if we have one in the future, maybe we can add a new field to the kernelspec in a dedicated PR?

minrk commented 1 year ago

That sounds reasonable to me, thanks!

JohanMabille commented 1 year ago

Here is a potential diff of the Kernel Protocol documentation when we add the Optional Features:

--- a/docs/messaging.rst
+++ b/docs/messaging.rst
@@ -246,6 +246,24 @@ block waiting for a reply, so the frontend must answer.
 Both sides should allow unexpected message types, and extra fields in known
 message types, so that additions to the protocol do not break existing code.

+.. _optional_features:
+
+Optional Features
+=================
+
+Optional features are parts of the protocol that kernels are not required to
+implement. An optional feature can be a list of additional messages and/or a
+list of additional fields in different existing messages. When a feature introduces
+new messages, it is its responsibility to specify the order of these messages when
+it makes sense. Under no circumstances this feature should alter the order of already
+existing messages, nor interleave new messages between already existing messages.
+
+Kernels can tell the frontend which optional features they support thanks to the
+``'supported_features'`` field in the :ref:`kernel_info_reply <msging_kernel_info>`.
+
+Available optional features are:
+- debugger: :ref:`debug request <debug_request>`, :ref:`debug event <debug_event>`
+
 .. _wire_protocol:

 The Wire Protocol
@@ -998,8 +1016,13 @@ Message type: ``kernel_info_reply``::

         # A boolean flag which tells if the kernel supports debugging in the notebook.
         # Default is False
+        # 'debugger' is considered as deprecated, prefer the 'supported_features' list
+        # instead
         'debugger': bool,

+        # Optional: The list of optional features supported by the kernel
+        'supported_features': [str],
+
         # Optional: A list of dictionaries, each with keys 'text' and 'url'.
         # These will be displayed in the help menu in the notebook UI.
         'help_links': [
@@ -1010,6 +1033,9 @@ Message type: ``kernel_info_reply``::
 Refer to the lists of available `Pygments lexers <http://pygments.org/docs/lexers/>`_
 and `codemirror modes <http://codemirror.net/mode/index.html>`_ for those fields.

+Refer to the :ref:`Optional Features section <_optional_features>` for the list of
+available optional features.
+
 .. versionchanged:: 5.0

     Versions changed from lists of integers to strings.
@@ -1106,8 +1132,16 @@ Message type: ``interrupt_reply``::

 .. versionadded:: 5.3

-Debug request
--------------
+.. _debug_request:
+
+Debug request (Optional Feature: debugger)
+------------------------------------------
+
+.. note::
+
+    This message is part of the debugger optional feature. Kernels should indicate
+    to the front end that they support it by appending the ``debugger`` string
+    to the ``supported_features`` field of the ``kernel_info_reply`` message.

 This message type is used with debugging kernels to request specific actions
 to be performed by the debugger such as adding a breakpoint or stepping into
@@ -1574,8 +1608,14 @@ Message type: ``clear_output``::

 .. _debug_event:

-Debug event
------------
+Debug event (Optional feature: debugger)
+----------------------------------------
+
+.. note::
+
+    This message is part of the debugger optional feature. Kernels should indicate
+    to the front end that they support it by appending the ``debugger`` string
+    to the ``supported_features`` field of the ``kernel_info_reply`` message.

 This message type is used by debugging kernels to send debugging events to the
 frontend.
JohanMabille commented 1 year ago

The kernel handshake pattern itself should not be optional as discussed in #66.

This point is still under discussion AFAK.

It's not, as stated in the JEP:

"This pattern is NOT a replacement for the current connection pattern. It is an additional one and kernels will have to implement both of them to be conformant to the Jupyter Kernel Protocol specification. Which pattern should be used for the connection is decided by the kernel launcher, depending on the information passed in the initial connection file."

The point that was discussed was wether that new pattern should replace the old one, and the conclusion was they should live together (there are use cases for which we cannot drop the old connection pattern).

fcollonval commented 1 year ago

The vote is now closed with the results:

In favor: 7 (Eric, Frederic, Itay, Johan, Min, Sylvain, Zach) Against: 0 Abstention: 0 No vote: 4 (Carol, Isabela, Paul, Rick)

--> In light of those results, this JEP is accepted.