Test downstream projects

[davidbrochart]

It looks like ipykernel master breaks nbclient. I reproduced locally and I can see messages like:

msg = {"shell": {...}, "control": {}}

where ... is a regular message (with its header and so on).
Any idea where this could come from?

[davidbrochart]

I think it comes from here:

ipykernel/ipykernel/comm/comm.py

Line 69 in 5d21c8f

parent=self.kernel._parent_header,

I don't know ipykernel, but it looks like we should choose a channel here, e.g.:

parent=self.kernel._parent_header["shell"]

[davidbrochart]

The last commit seems to fix this issue, but now there are other errors:

At index 0 diff:
{'ename': 'KeyboardInterrupt', 'evalue': '', 'output_type': 'error', 'traceback': ['---------------------------------------------------------------------------', 'KeyboardInterrupt                         Traceback (most recent call last)', '<IPY-INPUT> in <module>\n----> 1 while True: continue\n', 'KeyboardInterrupt: ']} !=
{'output_type': 'error', 'ename': 'KeyboardInterrupt', 'evalue': '', 'traceback': ['---------------------------------------------------------------------------', 'KeyboardInterrupt                         Traceback (most recent call last)', '/tmp/ipykernel_1789/2216019953.py in <module>\n----> 1 while True: continue\n', 'KeyboardInterrupt: ']}

It looks like the cell "file name" changed, and that is reflected in the traceback.
Is it related to the work done on the debugger?

[SylvainCorlay]

Release 6.0.0a4 with the latest fixes. It may be interesting to rebase this on master.

[SylvainCorlay]

It looks like the cell "file name" changed, and that is reflected in the traceback.
Is it related to the work done on the debugger?

These are expected changes due to the debugger support, which makes the traceback message a little bit different wrt filenames.

[davidbrochart]

These are expected changes due to the debugger support, which makes the traceback message a little bit different wrt filenames.

It would be good to support passing a path, e.g. path_to_my_cell_dir instead of having e.g. /tmp/ipykernel_647963/156288784.py that is kind of random. The path_to_my_cell_dir directory could be populated with incrementing file names for example, like 1.py, 2.py, etc. That would make testing easier.

[SylvainCorlay]

Indeed there are some test failures in ipyparallel. I don't know if these failures already occur with ipykernel stable.

cc @minrk

[davidbrochart]

I just tested ipykernel master with nbclient, and there are zmq.error.ZMQError: Socket operation on non-socket errors (see https://github.com/jupyter/nbclient/pull/146/checks?check_run_id=2450827376):

  E         + ERROR:tornado.application:Exception in callback functools.partial(<function ZMQStream._update_handler.<locals>.<lambda> at 0x7fc058f7fe50>)
  E         + Traceback (most recent call last):
  E         +   File "/home/runner/work/nbclient/nbclient/.tox/py38/lib/python3.8/site-packages/tornado/ioloop.py", line 741, in _run_callback
  E         +     ret = callback()
  E         +   File "/home/runner/work/nbclient/nbclient/.tox/py38/lib/python3.8/site-packages/zmq/eventloop/zmqstream.py", line 535, in <lambda>
  E         +     self.io_loop.add_callback(lambda: self._handle_events(self.socket, 0))
  E         +   File "/home/runner/work/nbclient/nbclient/.tox/py38/lib/python3.8/site-packages/zmq/eventloop/zmqstream.py", line 447, in _handle_events
  E         +     zmq_events = self.socket.EVENTS
  E         +   File "/home/runner/work/nbclient/nbclient/.tox/py38/lib/python3.8/site-packages/zmq/sugar/attrsettr.py", line 51, in __getattr__
  E         +     return self._get_attr_opt(upper_key, opt)
  E         +   File "/home/runner/work/nbclient/nbclient/.tox/py38/lib/python3.8/site-packages/zmq/sugar/attrsettr.py", line 63, in _get_attr_opt
  E         +     return self.get(opt)
  E         +   File "zmq/backend/cython/socket.pyx", line 464, in zmq.backend.cython.socket.Socket.get
  E         +   File "zmq/backend/cython/socket.pyx", line 135, in zmq.backend.cython.socket._check_closed
  E         + zmq.error.ZMQError: Socket operation on non-socket

[SylvainCorlay]

Rebased on 6.0.0a6.

[minrk]

For ipyparallel, can you test main? I've fixed a few things since the last alpha, but probably won't be able to make a release for some weeks. I might be able to make a quick prerelease, if that's easier.

I think I recall seeing a parent-header-related issue when I had master of ipykernel installed locally, relating to parent headers and the output widget, I believe. Not sure if that would be covered by widgets tests

[SylvainCorlay]

Now testing ipyparallel@main.

We are seeing the same failure as previously, with AssertionError: TaskAborted not raised by get.

[JohanMabille]

/tmp/ipykernel_647963/156288784.py that is kind of random

This is not random at all, the pattern is /tmp/ipykernel_{pid}/{murmurhash(cell_content)}.py

[minrk]

I've tracked down the abort test to the fact that the debugger is not enabled and the control thread is not running in ipyparallel, which is the default behavior of the Kernel object. The result is that control thread priority is not preserved, and the shell channel can now starve the control channel (rather than the other way around, which is the prior and correct behavior). I think this is an issue, and just need to consider options:

1. fix control channel priority when control thread is not running (most compatible, probably most complicated)
2. require control thread for control channel priority behavior (and launch control thread in IPP). Most backward-incompatible, but simplest.
   a. less backward-incompatible is to make instantiating a control thread the default behavior in Kernel itself, rather than KernelApp

I'll try to figure out what's involved in each and what makes the most sense. I'm guessing for maintenance purposes, ensuring the control thread is running might actually be the simplest way to go since there won't be two different ways for control and shell queues to interact.

[davidbrochart]

Looks like some jupyter_client tests fail.

[blink1073]

Kicking CI

[blink1073]

This is passing now, ready for review?

[davidbrochart]

I'm not sure we want to test ipykernel master with downstream packages' master, otherwise it might fail quite often.
Isn't the goal to test ipykernel master with downsteam packages' releases?

[SylvainCorlay]

I am in favor of merging this - even with an allow-failure tag, so that we are aware of unintentional breakage in ipykernel's API.

@davidbrochart Do you know the reason for the remaining failure?

[davidbrochart]

The remaining failures are in ipyparallel: fixture 'io_loop' not found.

[davidbrochart]

I think this is now ready to be (squashed and) merged.

[blink1073]

Excellent, thank you!