ipython / ipykernel

IPython Kernel for Jupyter
https://ipykernel.readthedocs.io/en/stable/
BSD 3-Clause "New" or "Revised" License
645 stars 365 forks source link

Confusing problems with `shutdown_reply` being received on iopub #170

Open gibiansky opened 8 years ago

gibiansky commented 8 years ago

Note #1: The IPython kernel sends a status: "ok" key with the shutdown_reply, which is not documented in the messaging spec. Extra keys are generally not a problem but could also be documented if they play any role (or it can also be documented that extra keys should never be rejected).

More serious issue:

I am getting the shutdown_reply sent on the iopub channel, which can break client libraries. This seems to be somewhat tricky to reproduce, which has me confused.

Here is how you I can reproduce it:

Start a kernel:

python -m ipykernel

Find the absolute path to kernel-DDDDD.json in the stdout of that process.

Then, run jupyter console, and enter the following code (substituting in kernel-DDDDD.json as appropriate):

from pprint import pprint
from jupyter_client.consoleapp import JupyterConsoleApp
import time

class MyKernelApp(JupyterConsoleApp):
    def __init__(self, connection_file, runtime_dir):
        self._dispatching = False
        self.existing = connection_file
        self.runtime_dir = runtime_dir
        self.initialize()

app = MyKernelApp("/Users/silver/Library/Jupyter/runtime/kernel-23605.json", "/tmp")
kc = app.kernel_client
# Clear status messages
time.sleep(0.5)
kc.iopub_channel.get_msgs()

kc.shutdown()
time.sleep(0.5)
[m["header"]["msg_type"] for m in kc.iopub_channel.get_msgs()]

This will result in a list of message types which include shutdown_reply, which does not make sense, because this is the iopub channel.

Similarly, the following code reproduces the problem for me:

multimanager = MultiKernelManager()
manager = multimanager.get_kernel(multimanager.start_kernel("python3"))
client = manager.client()
time.sleep(0.5)
print(client.iopub_channel.get_msgs())
client.shutdown()
time.sleep(0.5)
print([m["header"]["msg_type"] for m in client.iopub_channel.get_msgs()])

However, the following code does not reproduce the issue:

multimanager = MultiKernelManager()
manager = multimanager.get_kernel(multimanager.start_kernel("python3"))
client = manager.client()
client.shutdown()
print(client.get_shell_msg())
print(client.get_iopub_msg())

Instead, it just blocks on get_iopub_msg, after returning the shutdown_reply message on the shell channel.

Can you help me figure out what's going on? Is the shutdown_reply being sent on both channels? Why is it being received on iopub at all?

I am quite confused. (The real issue I'm having is on a Haskell codebase, but hopefully these code snippets are enough to reproduce it.)

Here are some versions:

Python 3.5.0 (default, Oct  3 2015, 11:20:34)
[GCC 4.2.1 Compatible Apple LLVM 7.0.0 (clang-700.0.72)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import ipykernel; print(ipykernel.__version__)
4.3.1
>>> import jupyter_client; print(jupyter_client.__version__)
4.3.0
gibiansky commented 8 years ago

To add yet another little bit of data, in the Haskell implementation I am receiving the shutdown_reply on both shell and iopub, with different times in the headers:

Iopub: {"session":"59cdd707-0a40-4e37-a3f7-096ea2e87d7c","date":"2016-08-02T23:28:31.346912","version":"5.0","msg_id":"43ee94bc-d874-4ce5-8857-506b18ddbc96","username":"silver","msg_type":"shutdown_reply"}
Shell: {"session":"59cdd707-0a40-4e37-a3f7-096ea2e87d7c","date":"2016-08-02T23:28:31.346758","version":"5.0","msg_id":"dab5b281-8ecc-4335-8c5c-d87f6aed17b9","username":"silver","msg_type":"shutdown_reply"}
gibiansky commented 8 years ago

Now that I am thinking about it, it would make sense if this were consistent and on purpose, to re-broadcast the shutdown to all connected clients.

And, now that I am looking at it, we discussed this a few weeks ago here: https://github.com/jupyter/jupyter_client/issues/170

So yes, this is the shutdown_reply just being rebroadcast consistently on iopub, and this simply needing to be documented in the spec.

In that case, this can be boiled down to two things:

minrk commented 8 years ago

Document the status key of the shutdown_reply

It is documented that all replies should have a status field, with the value either 'ok' or 'error' in most cases though this is rarely checked, so omitting it will likely not be noticed by clients. In fact, as I look at IPython, I see we are omitting it in a few places.

Document shutdown_reply on iopub (or a similar message) eventually, but the shutdown reply for the current messaging spec version

👍 while I would prefer replacing shutdown_reply with a dedicated IOPub message, documenting what we currently do is probably the right thing now.

gibiansky commented 8 years ago

Good point, I missed that!

Yes, it would be great if all the messages IPython sends followed that, but it seems pretty reasonable to just assume that status is "ok" unless otherwise specified.

minrk commented 8 years ago

Yes, it would be great if all the messages IPython sends followed that

understatement :) and should be fixed by https://github.com/ipython/ipykernel/pull/171

it seems pretty reasonable to just assume that status is "ok" unless otherwise specified.

that's what clients have done so far, and I suppose we can encourage them to do so.