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

Dropping the dependency to tornado #656

Closed
SylvainCorlay opened this issue May 6, 2021 · 2 comments · Fixed by #1079
Closed

Dropping the dependency to tornado #656

SylvainCorlay opened this issue May 6, 2021 · 2 comments · Fixed by #1079

Comments

@SylvainCorlay
Copy link
Member

We are not using the tornado coroutines anymore in the codebase (since #632) which also fixed some issue with respect to context variables. I think that it would make sense to drop tornado completely at some point.

I am opening this issue to track the work required for this, as it may require significant work in the codebase:

  • switching the asyncio API of pyzmq.
  • asyncio enforces stronger constraints about instanciating queues and loops in the thread where they are run.
@SylvainCorlay SylvainCorlay changed the title Dependency to tornado Dropping the dependency to tornado May 6, 2021
@minrk
Copy link
Member

minrk commented May 12, 2021

Adding a note that this is not a pressing issue - since we use tornado as very little more than an asyncio runner, we might as well run asyncio directly, but there is no real cost to continue using tornado as the runner at this point. It's fine to remove things piecemeal (native coroutines done, asyncio Queues next, etc.), but indeed the switch to zmq.asyncio will be the biggest change.

This is technically a user-facing change, in that anyone calling IOLoop.current() will no longer have access to a running loop, but I'm not sure anyone is actually doing that.

@SylvainCorlay
Copy link
Member Author

Absolutely. I opened the issue because I can't tackle that right now, however, it would be a nice thing to do in the future.

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

Successfully merging a pull request may close this issue.

2 participants