Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix asyncio error when opening notebooks #6221

Merged
merged 1 commit into from
Nov 16, 2021
Merged

Conversation

dleen
Copy link
Contributor

@dleen dleen commented Nov 3, 2021

This is a fix for #6164

nest_asyncio must be applied before any async tasks have been created
otherwise there could be tasks queued that are unpatched, and thus do
not respect nested loops. An example of an unpatched task can be seen in
the original issue:

<Task pending coro=<HTTP1ServerConnection._server_request_loop() running at /apps/python3/lib/python3.7/site-packages/tornado/http1connection.py:823>

which originates from Tornado.

A similar issue was reported in nest-asyncio: erdewit/nest_asyncio#22
where the solution is to call apply on import so that unpatched tasks
do not get created.

This is a fix for jupyter#6164

`nest_asyncio` must be applied before any async tasks have been created
otherwise there could be tasks queued that are unpatched, and thus do
not respect nested loops. An example of an unpatched task can be seen in
the original issue:

```
<Task pending coro=<HTTP1ServerConnection._server_request_loop() running at /apps/python3/lib/python3.7/site-packages/tornado/http1connection.py:823>
```
which originates from Tornado.

A similar issue was reported in `nest-asyncio`: erdewit/nest_asyncio#22
where the solution is to call `apply` on import so that unpatched tasks
do not get created.
Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good to me and I've confirmed the pending task error is not produced. Thank you.

I'd prefer to wait for another review prior to merging and have requested the help of others.

So we must call `nest_asyncio.apply()` method as early as possible. It
is preferable to do this in the consuming application rather than the
`jupyter_client` as it is a global patch and would impact all consumers
rather than just the ones that rely on synchronous kernel behavior.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice comment - thank you.

@blink1073 blink1073 added the bug label Nov 5, 2021
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you @dleen!

@dleen
Copy link
Contributor Author

dleen commented Nov 15, 2021

@minrk Are you able to take a look?

@minrk minrk merged commit 160c27d into jupyter:master Nov 16, 2021
@minrk
Copy link
Member

minrk commented Nov 16, 2021

Thanks!

yuvipanda added a commit to 2i2c-org/utexas-image that referenced this pull request Apr 8, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants