jupyter / notebook

Jupyter Interactive Notebook
https://jupyter-notebook.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
11.74k stars 4.99k forks source link

Asyncio error when opening notebooks #6164

Open ghost opened 3 years ago

ghost commented 3 years ago

When I open a notebook from /tree, i get the following error in terminal:

  return _iterencode(o, 0)
[W 20:44:29.469 NotebookApp] Notebook NLP/3.02-obtain-model-results.ipynb is not trusted
ERROR:asyncio:Exception in callback <TaskWakeupMethWrapper object at 0x000001D12EB68DC8>(<Future finis...5ea"\r\n\r\n'>)
handle: <Handle <TaskWakeupMethWrapper object at 0x000001D12EB68DC8>(<Future finis...5ea"\r\n\r\n'>)>
Traceback (most recent call last):
  File "c:\users\work\miniconda3\envs\playground\lib\asyncio\events.py", line 88, in _run
    self._context.run(self._callback, *self._args)
RuntimeError: Cannot enter into task <Task pending coro=<HTTP1ServerConnection._server_request_loop() running at c:\users\work\miniconda3\envs\playground\lib\site-packages\tornado\http1connection.py:823> wait_for=<Future finished result=b'GET /api/co...f5ea"\r\n\r\n'> cb=[IOLoop.add_future.<locals>.<lambda>() at c:\users\work\miniconda3\envs\playground\lib\site-packages\tornado\ioloop.py:688]> while another task <Task pending coro=<KernelManager._async_start_kernel() running at c:\users\work\miniconda3\envs\playground\lib\site-packages\jupyter_client\manager.py:336>> is being executed.

Desktop (please complete the following information):

kevin-bates commented 3 years ago

Hi @nissankarkifm - thanks for opening this issue.

I suspect this was introduced with the refactoring done in jupyter_client to use async methods for the synchronous KernelManager. The symptoms are the same as here but this is a bit beyond my knowledge level. Although the kernel/notebook appears to behave fine, there's probably something hung up somewhere.

On the bright side, this issue can be avoided by configuring Notebook to run with its AsyncMappingKernelManager instead - which introduces a full async stack.

jupyter notebook --NotebookApp.kernel_manager_class=notebook.services.kernels.kernelmanager.AsyncMappingKernelManager

Or configure via the configuration file:

## The kernel manager class to use.
#  Default: 'notebook.services.kernels.kernelmanager.MappingKernelManager'
c.NotebookApp.kernel_manager_class = 'notebook.services.kernels.kernelmanager.AsyncMappingKernelManager'

We should probably consider making this (async kernel management) the default at some point. This would require jupyter_client >= 6.x.

Another option would be to cap jupyter_client < 7, but you'll miss out on features exposed in jupyter_client 7+.

And a third option would be to switch to Jupyter Lab 3.x since it uses jupyter_server (which uses AsyncMappingKernelManager by default) instead of notebook for its server.

cc: @davidbrochart @maartenbreddels

ghost commented 3 years ago

@kevin-bates Changing the config worked for me. Thanks!

rayjennings3rd commented 3 years ago

Which configuration file should that line go in?

ghost commented 3 years ago

Referring to this document. https://jupyter-notebook.readthedocs.io/en/stable/config.html
I run this command jupyter notebook --generate-config
"The Jupyter folder is in your home directory, ~/.jupyter. jupyter_notebook_config.py "

meeseeksmachine commented 3 years ago

This issue has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/unable-to-run-jupyter-notebook-after-update-localhost/10764/2

ozancaglayan commented 3 years ago

Hello,

For me, the notebook/kernel seems to be running but the throbber of the browser's tab is stuck spinning forever. Applying the fix above, solves both the error and the throbber.

I think a new release should be done to fix this annoying issue.

meeseeksmachine commented 3 years ago

This issue has been mentioned on Jupyter Community Forum. There might be relevant details there:

https://discourse.jupyter.org/t/unable-to-run-jupyter-notebook-after-update-localhost/10764/8

NiklasRosenstein commented 3 years ago

I'm experiencing the same issue. Using the configuration suggested by @kevin-bates works. (Python 3.9, OSX, Jupyter Notebook 6.4.4)

Under what circumstances is the default configuration expected to work out of the box? I think this issue should be re-opened and only closed when running Jupyter notebook works right away without having to override this option.

kevin-bates commented 3 years ago

Under what circumstances is the default configuration expected to work out of the box? I think this issue should be re-opened and only closed when running Jupyter notebook works right away without having to override this option.

That's a fair comment. I think we have two options...

  1. Change the default multi-kernel manager to AsyncMappingKernelManager from MappingKernelManager - as has been the case in jupyter_server since the get-go essentially.
  2. Update (probably) jupyter_client sync-based methods to address the issue. (cc: @davidbrochart)

As I noted previously, this is getting into async details that I'm not adequately knowledgable about and perhaps David has some insights. For example, might https://github.com/jupyter/jupyter_client/pull/697 have a positive impact on this?

@NiklasRosenstein, to that last point, would you be able to install jupyter_client 7.0.4 (available just today) and see if this has a positive influence prior to deploying the workaround?

I'll go ahead and re-open this issue to your original point.

davidbrochart commented 3 years ago

I think option 1 is the most reasonable one. The error seems related to Tornado but I'm not sure how to fix it in jupyter_client (I don't think https://github.com/jupyter/jupyter_client/pull/697 will fix it). I know people love the Classic Notebook, but maybe it is time to switch to JupyterLab? Things will only get worse with Classic, since all the efforts are now going into Lab. And for people who don't like the UI of JupyterLab, you can get pretty much the same Classic Notebook experience with https://github.com/jupyterlab/retrolab.

tovrstra commented 3 years ago

Just as a side note: some extensions do not work yet for Jupyter Lab, e.g. RISE, making it worthwhile to keep Classic Notebooks working.

stojan-jovic commented 3 years ago

Workaround to set AsyncMappingKernelManager at startup works for me, but not on every notebook run (i.e. sometimes I need to close notebook and then open it again to avoid reported error and make kernel working). Sometimes it's little frustrating, especially when it happens two or more times in a row.

Btw. this is my versions stack:

ipykernel            6.4.2
ipython              7.28.0
ipython-genutils     0.2.0
ipywidgets           7.6.5
jupyter-client       7.0.6
jupyter-core         4.8.1
jupyter-server       1.11.1
jupyterlab           3.2.0
jupyterlab-pygments  0.1.2
jupyterlab-server    2.8.2
jupyterlab-widgets   1.0.2
nbclassic            0.3.2
nbclient             0.5.4
nbconvert            6.2.0
nbformat             5.1.3
nest-asyncio         1.5.1
notebook             6.4.5

Windows 10 Firefox 93.0

Are there plans to fix this, or we definitely should switch to Jupyter Lab and get used to it?

davidbrochart commented 3 years ago

or we definitely should switch to Jupyter Lab and get used to it?

RetroLab looks pretty much like Classic Notebook, hopefully you shouldn't be too bothered by the switch.

dleen commented 3 years ago

I also encountered this issue when upgrading to jupyter_client=7.x.x. I read all of the linked issues, especially https://github.com/erdewit/nest_asyncio/issues/22. The issue mentions:

it will be triggered when asyncio machinery wants to resume a task that is unpatched (i.e. created before .apply() was called).

This lead me to the code in jupyter_client/utils.py: https://github.com/jupyter/jupyter_client/blob/master/jupyter_client/utils.py

def run_sync(coro):
    def wrapped(*args, **kwargs):
        try:
            loop = asyncio.get_event_loop()
        except RuntimeError:
            loop = asyncio.new_event_loop()
            asyncio.set_event_loop(loop)
        import nest_asyncio  # type: ignore

        nest_asyncio.apply(loop)
        future = asyncio.ensure_future(coro(*args, **kwargs))
        try:
            return loop.run_until_complete(future)
        except BaseException as e:
            future.cancel()
            raise e

We are only applying nest_asyncio only after run_sync has been called which means there could be tasks queued that are unpatched e.g. from the original issue: <Task pending coro=<HTTP1ServerConnection._server_request_loop() running at /apps/python3/lib/python3.7/site-packages/tornado/http1connection.py:823> which looks like it originated from Tornado.

I make a quick change locally:

diff --git a/jupyter_client/utils.py b/jupyter_client/utils.py
index f2f3c4d..1cf590a 100644
--- a/jupyter_client/utils.py
+++ b/jupyter_client/utils.py
@@ -7,6 +7,9 @@ import asyncio
 import inspect
 import os

+import nest_asyncio
+nest_asyncio.apply()
+

 def run_sync(coro):
     def wrapped(*args, **kwargs):
@@ -15,7 +18,6 @@ def run_sync(coro):
         except RuntimeError:
             loop = asyncio.new_event_loop()
             asyncio.set_event_loop(loop)
-        import nest_asyncio  # type: ignore

         nest_asyncio.apply(loop)
         future = asyncio.ensure_future(coro(*args, **kwargs))

which applies nest_asyncio at import time allowing it to patch all tasks. And then we still patch the new loop as before.

This resolves the issue for me which I was easily able to reproduce. I also ran using the AsyncKernelManager both before and after this change and it ran fine.

@kevin-bates @davidbrochart are you convinced by this explanation? Would you accept this as a PR to Jupyter client?

kevin-bates commented 3 years ago

@dleen - thank you for looking into this! I'm convinced, but that's not saying a whole lot. I would prefer hearing from @davidbrochart since he's got this stuff more wired than myself.

Also note that the import statement was moved into its current position in https://github.com/jupyter/jupyter_client/pull/665, so we should probably run this past @SylvainCorlay as well.

~I'm going ahead and transferring this issue to jupyter_client since that's where this needs to be addressed anyway.~

Turns out I don't have transfer rights to that repo, so this will remain here for now.

dleen commented 3 years ago

Also note that the import statement was moved into its current position in jupyter/jupyter_client#665, so we should probably run this past @SylvainCorlay as well.

Yeah I had noticed that and I wasn't sure if it was a functional change, or just a housekeeping/linting/style thing (no commit message). But the important thing is that it wasn't calling apply either. nest-asyncio doesn't have any side effects on import.

dleen commented 3 years ago

I opened a PR so we can continue the discussion there if it's better: https://github.com/jupyter/jupyter_client/pull/718

kevin-bates commented 3 years ago

Thanks David. We'll keep this issue open until we finalize things a bit.

davidbrochart commented 3 years ago

Thanks @dleen, I commented on your PR. But given that the future of the Classic Notebook is taking a direction that will probably solve this issue (by using RetroLab for Notebook v7), I'm not sure we should try and fix Notebook<7.0.

kevin-bates commented 3 years ago

Hmm. It will be several months before Notebook 7 is available given there are several "critical" extensions that require migration to the lab framework. In addition, there will be folks that never move to Notebook 7 because their "critical" extensions are not available.

Are we convinced that moving the AsyncMappingKernelManager resolves this issue? If so, perhaps we can introduce a flag (e.g., --use-async-kernel-management) that makes it easier for folks to change the configuration. Or open discussion about flipping the default in a minor release (6.5) - which, I'm guessing, could be considered unconventional. However, there could still be users for which this doesn't work (like those have implemented their own synchronous kernel manager) so I'm not convinced this is a final solution.

Another approach would be to apply the patch in _init_asyncio_patch() as done for https://github.com/jupyter/notebook/pull/6052. Also note that notebook has its own run_sync utility.

That all said, I'd love to get @minrk's opinion on this.

dleen commented 3 years ago

I followed @kevin-bates suggestion of applying the patch in an existing location where we patch asyncio _init_asyncio_patch(). Appreciate all the feedback! https://github.com/jupyter/notebook/pull/6221

I also explored if the notebooks own run_sync utility could work in jupyter_client but it did not.

dleen commented 3 years ago

But given that the future of the Classic Notebook is taking a direction that will probably solve this issue (by using RetroLab for Notebook v7), I'm not sure we should try and fix Notebook<7.0.

@davidbrochart I definitely agree with you here! I actually hit this issue while upgrading jupyter_client to 7.x.x (we have currently pinned jupyter_client<7 which I'm working on removing). We are migrating to Lab/RetroLab and part of that process is getting all of the dependencies to their latest upstream version.

maddukurisrinivasarao commented 2 years ago

Hmm. It will be several months before Notebook 7 is available given there are several "critical" extensions that require migration to the lab framework. In addition, there will be folks that never move to Notebook 7 because their "critical" extensions are not available.

Are we convinced that moving the AsyncMappingKernelManager resolves this issue? If so, perhaps we can introduce a flag (e.g., --use-async-kernel-management) that makes it easier for folks to change the configuration. Or open discussion about flipping the default in a minor release (6.5) - which, I'm guessing, could be considered unconventional. However, there could still be users for which this doesn't work (like those have implemented their own synchronous kernel manager) so I'm not convinced this is a final solution.

Another approach would be to apply the patch in _init_asyncio_patch() as done for #6052. Also note that notebook has its own run_sync utility.

That all said, I'd love to get @minrk's opinion on this.

is it safe to use the workaround suggested above in this thread (i.e c.NotebookApp.kernel_manager_class = 'notebook.services.kernels.kernelmanager.AsyncMappingKernelManager')?

my version stack: ipykernel 5.5.5 ipyparallel 6.2.4 ipython 7.25.0 ipython-genutils 0.2.0 ipywidgets 7.6.5 jupyter 1.0.0 jupyter-client 7.0.6 jupyter-console 6.4.0 jupyter-core 4.9.1 jupyter-packaging 0.11.0 jupyter-sphinx 0.3.2 notebook 6.4.5 nbconvert 5.6.1 nbformat 5.1.3 nest-asyncio 1.5.1

etejedor commented 2 years ago

is it safe to use the workaround suggested above in this thread (i.e c.NotebookApp.kernel_manager_class = 'notebook.services.kernels.kernelmanager.AsyncMappingKernelManager')?

I'd like to know the same thing too. We are moving to JupyterLab soon in our online notebook service but in the meantime we are getting hit by this error in user sessions every now and then.

kevin-bates commented 2 years ago

Moving to asynchronous kernel management is probably safer than not since that's where the dev focus has been the last couple of years and JupyterServer has been using it since its inception (and on which Lab 3+ relies). So you're essentially moving your "experience" closer to where you want to be eventually.