Revert latest changes to `ThreadedZMQSocketChannel` because they break Qtconsole

jupyter / jupyter_client

Jupyter protocol client APIs

https://jupyter-client.readthedocs.io

BSD 3-Clause "New" or "Revised" License

383 stars 283 forks source link

Revert latest changes to `ThreadedZMQSocketChannel` because they break Qtconsole #803

Closed ccordoba12 closed 2 years ago

ccordoba12 commented 2 years ago

Those changes were done in commit https://github.com/jupyter/jupyter_client/commit/0057186519ed356b822b1b1a0052e4068fccff12, released in version 7.3.2.
They break Qtconsole and hence Spyder, but I checked locally that a simple revert fixes the problem.
I added the qtconsole test suite here to avoid future breakages.

ccordoba12 commented 2 years ago

@blink1073, this is ready for review. As you can see, the Qtconsole test suite passes with these changes (you can check that it dies with Jupyter-client 7.3.3).

Note: I don't know how to fix the MyPy error, so I need help with that.

blink1073 commented 2 years ago

Hi @ccordoba12, I think this might be an error in the typings in pyzmq. I added a type ignore for now.

blink1073 commented 2 years ago

Docs failure is unrelated, I'll pick that up in a separate PR.

ccordoba12 commented 2 years ago

I think this might be an error in the typings in pyzmq. I added a type ignore for now.

Great! Thanks for your help with that.

Thanks very much for adding the qtconsole test!

No prob. Jupyter-client is critical for us, so that's something we really needed to do (I'll do the same in IPykernel once I have some time).

blink1073 commented 2 years ago

I'll cut a release once I merge https://github.com/jupyter/jupyter_client/pull/804

davidbrochart commented 2 years ago

Thanks @blink1073 and @ccordoba12 for taking care of that. Something is still not clear to me, is the callback passed to ZMQStream.on_recv an awaitable or not in pyzmq v23.0? I raised that point in the original PR. Would love some feedback from @minrk.