Closed smacke closed 11 months ago
I don't think we want to go back to nest-asyncio, which has its own issues too. Maybe the long-term solution will be to allow nested event loops in asyncio, see https://github.com/python/cpython/issues/66435.
Maybe a dumb question, but since the issue seems to be related to Tornado, has anyone tried to run Jupyter Notebook with Jupyverse?
I don't think we want to go back to nest-asyncio, which has its own issues too.
Agreed (at least for the long term), the purpose of this PR is mainly to raise awareness around https://github.com/jupyter/notebook/issues/6721 -- I've found that this is usually the best way to pester more knowledgeable folks :)
Maybe a dumb question, but since the issue seems to be related to Tornado, has anyone tried to run Jupyter Notebook with Jupyverse?
Not sure (I haven't tried this) -- I only know that right now, for many people, if they do pip install --upgrade jupyter
to get the latest versions of dependencies, and run the "legacy" notebook v6 via jupyter notebook
, they get errors like https://github.com/jupyter/notebook/issues/6721
@davidbrochart I didn't know about Jupyverse (and it looks interesting) but I'm not able to run it because of the following dependency conflict (it seems that asphalt
used by the api requires an older version of typeguard
, asphalt's requirement here)
There are incompatible versions in the resolved dependencies:
typeguard>=3.0.2 (from pandera==0.15.1->-r requirements.in (line 19))
typeguard~=2.0 (from asphalt==4.11.1->jupyverse-api==0.1.6->jupyverse[auth,jupyterlab]==0.1.6->-r requirements.in (line 14))
I don't have this conflict locally. Which package depends on pandera
?
@davidbrochart Sorry I wasn't clear - (and this is off topic for this Issue probably). My large notebooks that have experienced the crashes in https://github.com/jupyter/notebook/issues/6721 use pandera to validate data at load time, it's not a dependency of anything in this repo or Jupyverse 🙇🏻 . The conflict meant that I couldn't follow up on your suggestion to try that server.
Anyway for Jupyter Notebook, a new Jupyverse plugin should be created. But if Jupyter Notebook v7 is fine for you, this will be easy since Jupyverse already has a RetroLab plugin, which should be very similar.
That's great that we have some workarounds. However, I am concerned that the default jupyter installation (with notebook v6) is currently broken by default. E.g. if I create a fresh virtualenv and do:
pip install --upgrade jupyter
jupyter notebook
I can reliably reproduce the issue from https://github.com/jupyter/notebook/issues/6721.
One way to reproduce is to generate a plot in a cell with matplotlib, and then hold down ctrl+enter to keep submitting the cell as fast as possible. This leaves the notebook in an unusable state.
While that particular kind of interaction may be unlikely to occur in practice, others are reporting that restart + run all followed by normal use will trigger it.
And it definitely root causes to this threaded run_sync
.
Sorry for chiming in on a PR but I didn't see any actual issue about this on here and did want to re-raise this. As you can see from https://github.com/jupyter/notebook/issues/6721 this is a major issue that is biting quite a few people. While we have put mitigation in place for our projects we still see this issue crop up quite frequently causing major issues for many Panel and HoloViews users. As of right now we can only recommend users pin older versions of jupyter-server and jupyter-client, which does not seem a reasonable long-term solution. Is there anything we can do to work towards a fix here?
I think the right solution is for notebook<7 to pin, as in this PR: https://github.com/jupyter/notebook/pull/6749. We can't undo the changes in the stdlib that drove us to move away from nest_asyncio
.
I recently found out about greenletio. It looks interesting, especially the ability to use await inside a sync function:
from greenletio import await_
async def async_function():
pass
def sync_function():
await_(async_function())
I agree, it does look promising, we're looking at using it for async PyMongo as well: https://gist.github.com/blink1073/3873afd23977a8ab94a423127f2ca2ce
The only thing is that it uses greenlet, which is a C extension and probably does some low-level magic, but it works on CPython. Not sure about PyPy though?
Good point, I think greenletio would need to support continuelet as well: https://doc.pypy.org/en/latest/stackless.html.
Ah, good, greenletio is tested on pypy: https://github.com/miguelgrinberg/greenletio/blob/db38bd2c5315cd0b9d515665f19256424fe655ee/.github/workflows/tests.yml#L25
Nice!
We can't undo the changes in the stdlib that drove us to move away from
nest_asyncio
.
I'd be keen to learn more about this -- has anybody written a tl;dr documenting some of these issues? I came across this extremely long thread but find myself more confused than when I started.
I don't know of a good tl;dr: here's the thread I was involved in: https://github.com/python/cpython/issues/93453#issuecomment-1279979052
I don't know of a good tl;dr: here's the thread I was involved in: https://github.com/python/cpython/issues/93453#issuecomment-1279979052
Thanks for the pointer!
However, I am concerned that the default jupyter installation (with notebook v6) is currently broken by default.
I happened to reread this several weeks later, and am now cringing hard. I hope folks on this thread will accept my apologies for the wording. I shouldn't have minimized the (incredible) contributions of Jupyter core contributors, especially not without context behind some of the changes being made.
Actually greenletio
would not help us running async code from a sync function running in an event loop:
import asyncio
from greenletio import await_
async def async_function():
pass
def sync_function():
await_(async_function())
async def main():
sync_function()
asyncio.run(main())
# RuntimeError: await_ cannot be called from the asyncio task
No, you'd have to have a synchronous class/function hierarchy, and then a parallel async version.
That's what we plan to do for PyMongo.
We believe #362 fixes the issue, but in the mean time Notebook 6.5.5 has pinned its use of jupyter_core
. https://github.com/jupyter/notebook/issues/6721#issuecomment-1662440259. I'm going to close this PR, since we don't want to go back to using nest_asyncio
.
We believe https://github.com/jupyter/jupyter_core/pull/362 fixes the issue
One way to reproduce is to generate a plot in a cell with matplotlib, and then hold down ctrl+enter to keep submitting the cell as fast as possible. This leaves the notebook in an unusable state.
Unfortunately, I can confirm that the above repro still reproduces the same issue on notebook v6 with the newest version of jupyter_core. Here are the (potentially relevant) libraries I'm using:
jupyter_core==5.3.2 jupyter_client==8.3.1 tornado==6.3.2 notebook==6.5.6
Though, maybe it is not really an issue anymore given the aforementioned pinning of jupyter_core to an older version in notebook v6, or given that notebook v7 is out.
This PR appears to fix the issue described in https://github.com/jupyter/notebook/issues/6721, whereby users sporadically encounter
zmq message arrived on closed channel
errors in the tornado loop. It reverts the threaded implementation ofjupyter_core.utils.run_sync
and replaces it with the original one based onnest_asyncio
.Actually it uses a patched version of
nest_asyncio
from here, due to portability issues with the original version.Unfortunately I don't fully understand why reverting the threaded implementation of
run_sync
is needed to eliminate these errors, but some users suggest it could have something to do with tornado coroutines not playing nicely when run across different threads: https://github.com/tornadoweb/tornado/issues/2871#issuecomment-638577266Test plan: did an editable install locally and verified everything works well, and that all the zmq errors I encountered before are gone.