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

free wish: tornado upgrade or switch to asyncio? #1124

Open
ltalirz opened this issue Mar 16, 2020 · 9 comments
Open

free wish: tornado upgrade or switch to asyncio? #1124

ltalirz opened this issue Mar 16, 2020 · 9 comments
Labels

Comments

@ltalirz
Copy link
Contributor

ltalirz commented Mar 16, 2020

Hi, we have an applicant for Google Summer of Code (@unkcpz) who is interested in working on upgrading the outdated tornado dependencies of AiiDA and, by extension, circus.
One question we have in this context: Are there any strong preferences for either upgrading to a more recent tornado version or getting rid of tornado entirely in favor of the asyncio module?

Any insights you may have on this question would be appreciated.

Tagging @sphuber @muhrin for info

@sphuber
Copy link
Contributor

sphuber commented Mar 17, 2020

@k4nar are you the person in charge for design decisions in circus now? If not, do you know who we should ask what direction you guys would like to take?

@k4nar
Copy link
Contributor

k4nar commented Mar 17, 2020

I don't know if there is a "person in charge" :) . The initial authors of the projects haven't contributed to it since a few years. I'm still doing some reviews & the releases, but not much more.

So I would say that we can take a decision collectively, based on facts.

I don't have a strong opinion on this. I think the right way to go is the path requiring the less amount of changes. Note that starting with the version 5, Tornado uses the asyncio event loop when available. So we could probably keep most of the current Tornado code while benefiting from asyncio.

However, the big question is what version of Python we want to support. Supporting only modern versions (Python 3.6+ ?) would probably make things a lot easier. This would make sense as older ones are not supported anymore, but I guess you will always have people complaining as they are stuck with old versions. As Circus is a system tool it's hard to install it with Python versions not supported by your OS.

@sphuber
Copy link
Contributor

sphuber commented Mar 17, 2020

Thanks for the comments @k4nar . As @ltalirz mentioned in the OP we are planning to migrate our code aiida-core and one of our libraries plumpy from using tornado to using asyncio. The reason we are doing this is because we have now dropped python 2 support making the built in module asyncio available. Since we are only using tornado for its IOLoop, we feel that it is better to just use asyncio. This reduces the number of direct dependencies and hopefully should reduce maintenance and compatibility problems. The fact that tornado itself also just defaults to asyncio suggests that they also think that is the best option if possible.

With respect to the support of various python versions: we maintain support for python versions as long as they are not EOL. That means that currently we support python 3.5 and up. From September 2020 we will start to drop 3.5 when it reaches EOL.

As far as I can tell (but I am no expert), circus also mostly uses tornado for its event loop functionality. Unless you feel really strongly about supporting python < 3.5, I think the best course of action is to drop tornado and just switch to asyncio. These versions of python are not even supported by the python organization itself anymore, so it becomes hard to justify to maintain compatibility for libraries such as circus.

@k4nar
Copy link
Contributor

k4nar commented Mar 17, 2020

This makes perfect sense to me. I think we should make a first PR to remove the support for every Python versions before 3.5, and then start the work to move to asyncio. If removing completely proves to be harder than expected, we always have the solution to fallback to Tornado.

If you are ok to work on this, that would be awesome! You can also ping @biozz who's been quite involved in the project lately.

@biozz
Copy link
Contributor

biozz commented Mar 17, 2020

I agree with the asyncio approach and dropping support for older python versions.

Please let me know if there is anything I can help with, I am willing to do so.

@thedrow
Copy link

thedrow commented Jun 4, 2020

May I suggest using AnyIO instead?

That way, Celery 5+ will be able to use circus with the trio event loop.

@unkcpz
Copy link
Contributor

unkcpz commented Jun 13, 2020

Thanks @thedrow for suggestion, but I am not quit understand of your requirement. As I know, the celery is another process manager which can be thought of as parallel to circus. Could you describe in more detail why you need AnyIO, if it provides significant benefits I assume rely on the standard libraries such as tornado and asyncio is preferable.

@sphuber
Copy link
Contributor

sphuber commented Jun 15, 2020

@thedrow part of the reasoning behind this issue and the PR that @unkcpz is preparing is too reduce the maintenance load on this project. The original authors are no longer working on it, and although some activity has returned, there has been little manpower available. I am not a maintainer, but I would be very hesitant to add more dependencies that just enable it to be used in conjunction with various clones of a Python built-in module. I think having circus depend only on asyncio will do its stability and maintainability a great favor.

@thedrow
Copy link

thedrow commented Jun 15, 2020

Thanks @thedrow for suggestion, but I am not quit understand of your requirement. As I know, the celery is another process manager which can be thought of as parallel to circus. Could you describe in more detail why you need AnyIO, if it provides significant benefits I assume rely on the standard libraries such as tornado and asyncio is preferable.

If I were to design Celery today (which we're actively doing in Celery 5+) I'd pick a library that can already manage subprocesses, bind sockets etc. such as Circus.

Celery is going to use trio as its event loop.

AnyIO is a library that allows you to pick your event loop.
If Cirrus wants to also be a flexible library as it previously were, I think we should allow our users to pick and choose the event loop they want to work with.

The Cirrus CLI can use whatever you wish it to.

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

No branches or pull requests

6 participants