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

tornado.platform.twisted disappeared but did not explain its replacement #2636

Closed
glyph opened this issue Mar 29, 2019 · 3 comments · Fixed by #2653
Closed

tornado.platform.twisted disappeared but did not explain its replacement #2636

glyph opened this issue Mar 29, 2019 · 3 comments · Fixed by #2653

Comments

@glyph
Copy link

glyph commented Mar 29, 2019

Many years' worth of documentation explains that folks can do

from tornado.platform.twisted import install
reactor = install()

I can see that in 004de9c#diff-77b5a8a33248ef0bcafbc1bb71e9f013 Twisted integration was removed, since we can all depend on the stdlib loop APIs. This is great, but it also breaks a bunch of Jupyter notebooks, tutorials, etc.

Could you be convinced to replace all those sprawling APIs with something like this:

def install():
    from twisted.internet.asyncioreactor import install
    install()
    from twisted.internet import reactor
    reactor.startRunning()
    return reactor

possibly with a warnings.warn explaining that users could just call these APIs directly, if that's the desired end-state?

@bdarnell
Copy link
Member

To be honest I didn't realize anyone but me ever used this module, until your message to async-sig a few days ago :) I can't remember the last time anyone asked any questions about it, so I assumed that indicated a lack of interest.

I also thought that install() had stopped working on python 3 in Tornado 5.0 (without anyone complaining) so it was safe to remove the python 2-only functionality in Tornado 6 (this is why I failed to add deprecation warnings in addition to the doc comments). But now that I think about it some more, this was wrong. TwistedIOLoop was no longer usable in Tornado 5+Python 3, but TornadoReactor and install() would (I think) still work.

If there's demand for it, I think I'm fine with keeping tornado.platform.twisted.install around indefinitely using asyncioreactor instead of TornadoReactor.

@glyph
Copy link
Author

glyph commented Apr 8, 2019

Thanks @bdarnell ! That would definitely work for me.

bdarnell added a commit to bdarnell/tornado that referenced this issue May 5, 2019
This function had more usage than I was aware of and we can preserve
the old behavior by making it an alias for asyncioreactor.install.

Fixes tornadoweb#2636
bdarnell added a commit to bdarnell/tornado that referenced this issue May 5, 2019
This function had more usage than I was aware of and we can preserve
the old behavior by making it an alias for asyncioreactor.install.

Fixes tornadoweb#2636
@glyph
Copy link
Author

glyph commented Jul 16, 2019

Thanks a ton for doing this!

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