NotebookClient.wait_for_reply hangs with jupyter_client 8 or later

[cscheid]

(I apologize in advance if this is a jupyter_client bug instead!)

Over at https://github.com/quarto-dev/quarto-cli, we are seeing a consistent hang when NotebookClient.wait_for_reply is called: quarto-dev/quarto-cli#4122

Here are steps for a minimal reproduction. I've tested this on macOS with Python 3.10.9, but we're seeing this issue on github actions in Ubuntu runners as well.

$ python -m venv venv
$ . ./venv/bin/activate
$ cat requirements.txt
jupyter
nbconvert
ipykernel
$ pip install --upgrade setuptools pip
$ pip install -r requirements.txt
$ cat repro.py
from nbclient import NotebookClient
import nbformat
import json

nb_input = json.dumps({
  'cells': [],
  'metadata': { 'kernelspec': {'name': 'python3'} },
})

client = NotebookClient(nbformat.reads(nb_input, as_version = 4))
client.create_kernel_manager()
client.start_new_kernel()
client.start_new_kernel_client()
info_msg = client.wait_for_reply(client.kc.kernel_info())
$ python repro.py

At this point, the process will hang. If, instead, one sets up the venv with this alternative requirements-no-jupyterclient8.txt file which pins jupyter_client to before version 8, then repro.py doesn't hang:

$ cat requirements-no-jupyterclient8.txt
jupyter
nbconvert
ipykernel
jupyter_client < 8.0.0

[cscheid]

Ok, I think I see the issue. What's happening is that jupyter_client 8's client.kc.kernel_info()) returns a Future rather than a str, and notebookclient doesn't know what to do with it.

I think it would be ideal for notebook client to either itself await for it (but that would mean converting all of nbclient to support asyncio as well), or to validate that the parameter passed has the right type (I suspect what's going on is that something is automatically serializing the future to a str, and ending up waiting for the wrong uuid).

[edublancas]

Our CI also started failing, and we narrowed it down to jupyter_client 8 (we run some of our examples with papermill, which uses jupyter_client, just like nbclient).

we didn't try downgrading to jupyter_client 7 since we already built an in-house notebook executor, and migrating to it fixed the issues.

[blink1073]

Ah, what is happening here is that by default NotebookClient explicitly uses AsyncKernelManager, which uses AsyncKernelClient, and now returns a Future from kernel_info, so you have to call client.async_wait_for_reply. I think what we should do instead is have NotebookClient.wait_for_reply accept futures as well.

@davidbrochart what do you think?

[blink1073]

Actually the error is in jupyter_client, I'm working on a fix in jupyter/jupyter_client#925