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

Remove usages of tornado io_loop in favor of native asyncio #1362

Open
blink1073 opened this issue Nov 16, 2023 · 4 comments
Open

Remove usages of tornado io_loop in favor of native asyncio #1362

blink1073 opened this issue Nov 16, 2023 · 4 comments

Comments

@blink1073
Copy link
Collaborator

blink1073 commented Nov 16, 2023

Problem

Tornado's io_loop is now a wrapper around the native asyncio loop, and the tornado docs recommend using asyncio and asyncio.run() directly. In addition, there are plans to deprecate and remove asyncio.set_event_loop_policy, meaning that the we would have to use the loop_factory parameter that was added in Python 3.12.

We still want to use the SelectorEventLoop on Windows when using tornado, because even though it supports ProactorEventLoop as of tornado 6.1, it is less efficient, because it is using selectors in a thread.

Because ServerApp.io_loop is a public property, we'll have to bump to 3.0, because it is used by at least jupyterlab in a couple of places in tests (see below). We may also want to change it so that we start the event loop earlier, maybe in launch_instance.

Proposed Solution

We should remove usages of tornadio.io_loop up and down the stack, and add convenience function(s) in jupyter_core.utils to handle starting an event loop for different versions of Python, and perhaps a replacement for PeriodicCallback.

I will start by working on a PR to jupyter_client that replaces usages of ZMQStream.

Additional context

Known usages:

  • jupyter_server: add_callback_from_signal, add_callback, start, and stop, all internal usages, but io_loop is technically a public property.
  • jupyter_client: uses ZMQStream , which we'll need a replacement for. This seems to be the biggest remaining lift.
  • ipykernel: will be partially removed by Replace Tornado with AnyIO ipython/ipykernel#1079, but there are still usages in jupyter_client/ioloop.
  • jupyterlab: browser_check.py and jupyterlab/tests/test_app.py both use app.io_loop
  • qtconsole: we'll be able to remove the _init_asyncio_patch shim once it depends on a version of jupyter_client that doesn't use ZMQStream in kernel_client.start_channels().
Copy link

welcome bot commented Nov 16, 2023

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@blink1073
Copy link
Collaborator Author

Actually, we could still expose an ioloop object but have it be the asyncio loop, so you have an easy way to get the current running loop without calling a convenience function. We wouldn't have to change the usages in jupyterlab, but it would still be a breaking change.

@minrk
Copy link
Contributor

minrk commented Nov 24, 2023

👍 to switching all internal use to asyncio. There's little to no cost in keeping app.io_loop as a reference to IOLoop.current(), though (can be a deprecated property), so I don't think a breaking change is required, unless we change when that attribute is defined (we may), but it would still avoid ~all unnecessary breakage.

@blink1073
Copy link
Collaborator Author

Good idea, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants