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

allow setting event loop policy #4184

Closed
wants to merge 2 commits into from
Closed

Conversation

minrk
Copy link
Member

@minrk minrk commented Nov 8, 2018

can be an import string to a class or an instantiated policy

additionally, avoid calling deprecated ioloop.install with pyzmq 17. This can complicate tornado eventloop instantiation.

closes #4183 as best I can think of, since I don't think we can in general load server extensions early enough that they could do this themselves.

from zmq.eventloop import ioloop
ioloop.install()
else:
ioloop = None
Copy link
Member

Choose a reason for hiding this comment

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

I'd be inclined to put the from tornado import here directly, so you can see both cases in one snippet.

policy = change.new
if isinstance(policy, str):
# if it's a str, it should be an import string
policy = import_item(policy)()
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't the Type traitlet allow importable strings already? Are there cases where it's important to pass an instance so you can instantiate it with non-default arguments?

@takluyver
Copy link
Member

More importantly, is this actually useful? The problem motivating it in #4183 isn't really solved by making it configurable - as you point out, extensions are loaded too late to influence creating the event loop. @mrocklin has a workaround for dask (dask/distributed#2343), and I'm not aware of anyone else who has asked for this.

How the event loop is created seems like squarely an application concern, so I'm -1 on making it configurable unless there's a clear use case that's not better solved another way. The more bits that can be configured, the harder it is to change the underlying code later.

@minrk
Copy link
Member Author

minrk commented Nov 16, 2018

While putting this together, I did realize that jupyter_notebook_config.py can also just do:

asyncio.set_event_loop_policy(Policy())

Which isn't really any more complex than

c.NotebookApp.event_loop_policy = Policy()

The main difference would be that having it in config makes it explicit that users can set the event loop policy, while the other just happens to work.

I think we should allow users to request running with uvloop, etc.

@blink1073
Copy link
Contributor

Closing this one as stale, cheers!

@blink1073 blink1073 closed this Jun 7, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2021
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.

Hang when setting an asyncio event loop policy in server extension
3 participants