[Deluge] #3207: Core: migrate from Twisted to Asyncio

[Deluge]

#3207: Core: migrate from Twisted to Asyncio
-----------------------------+----------------------------
Reporter: andar | Owner: andar
Type: feature-request | Status: new
Priority: major | Milestone: needs verified
Component: Core | Version: develop (git)
Keywords: |
-----------------------------+----------------------------
We should migrate away from the use of Twisted within Deluge, starting
with the Core.
asyncio[0] is available as a standard library with the async/await
keywords syntax as of Python 3.5. The move to asyncio will help simplify
and modernize the code-base, and also allow us to remove a major
dependency.

There shouldn't be anything Twisted specific that we actually need over
what is provided within asyncio, so it should be a fairly straight-forward
conversion.

A small example of how the code would differ:
https://asyncio.readthedocs.io/en/latest/twisted.html


Some thoughts:

* The async/await keywords were added in Python 3.5 which means we would
drop support for older versions of Python, I'd even suggest bumping the
requirement to 3.6 so that we can use format strings. In my opinion,
support for Python 2 should be dropped in the develop branch and for
Deluge 2.0 to simplify the code and reduce the maintenance cost.

* This ticket only addresses moving the Core to asyncio for the time-
being. I think this would be a first good step with the UIs to follow.
Some care will need to be made to make sure that the
DelugeTransferProtocol is available to both core and UI as it currently
stands and that plugins are also unaffected. I believe I should be able to
do this in a way that would allow the UIs to continue to use Twisted with
a migrated Core.

* We may have to add an additional dev dependency for asynctest[1] as the
standard library does not have some helpful additions this library
provides.


Any objections? Anything I didn't think about that may cause a problem?

[0] - https://docs.python.org/3/library/asyncio.html

[1] - https://github.com/Martiusweb/asynctest

--
Ticket URL: <https://dev.deluge-torrent.org/ticket/3207>
Deluge <http://deluge-torrent.org/>
Deluge Project

[Deluge]

#3207: Core: migrate from Twisted to Asyncio
------------------------------+---------------------------
Reporter: andar | Owner: andar
Type: feature-request | Status: new
Priority: major | Milestone: 2.0.0
Component: Core | Version: develop (git)
Resolution: | Keywords:
------------------------------+---------------------------
Changes (by andar):

* milestone: needs verified => 2.0.0


--
Ticket URL: <https://dev.deluge-torrent.org/ticket/3207#comment:1>
Deluge <http://deluge-torrent.org/>
Deluge Project

[Deluge]

#3207: Core: migrate from Twisted to Asyncio
------------------------------+---------------------------
Reporter: andar | Owner: andar
Type: feature-request | Status: new
Priority: major | Milestone: 2.0.0
Component: Core | Version: develop (git)
Resolution: | Keywords:
------------------------------+---------------------------

Comment (by Cas):

Yeah I agree moving from Twisted would be useful in the long term. It's a
lot of work though as it's integrated into everything, how much do we gain
from asyncio?

I don't think moving to Python 3.6 for f-strings is worth the hassle,
users likely still need 3.5. I have been targetting minimum Python 3.4 but
it is EOL soon so 3.5 is fine and almost there to deprecate/drop Python
2.7.

I am looking to release 2.0.0 soon so this ticket should be for a future
milestone. However if we can keep backward compatibility it shouldn't be
an issue for future point releases.

Some questions that come to mind:

- We will need to replace httpdownloader with perhaps requests lib?
- What will we replace web server with?
- How will we integrate asyncio with GTK3? What about Windows?

--
Ticket URL: <https://dev.deluge-torrent.org/ticket/3207#comment:2>
Deluge <http://deluge-torrent.org/>
Deluge Project

[Deluge]

#3207: Core: migrate from Twisted to Asyncio
------------------------------+---------------------------
Reporter: andar | Owner: andar
Type: feature-request | Status: new
Priority: major | Milestone: 2.0.0
Component: Core | Version: develop (git)
Resolution: | Keywords:
------------------------------+---------------------------

Comment (by andar):

IMHO, the biggest gain from using asyncio over Twisted is more readable
code. The use of deferreds and the nested functions all over the place
would go away. Using the await keyword makes things read more like
synchronous code.

In regards to the amount of work, I don't think it will be that bad for
the Core. It will mostly just converting functions to coroutines and
replacing d.addCallbacks, etc.. with awaits. The UIs could continue to use
Twisted if it will be too much work to convert them.

Twisted is overkill for what we actually need in Deluge, it just happened
to be the only library at the time to offer the things we did need.

We could use aiohttp[0] for both httpdownloader and the web server.

For GTK3, we could use gbulb[1], but I'm not sure of the Windows support.
If we need to stay with Twisted for GTK ui, that's OK, but it'd be nice to
convert at some point. We should also consider that the future of Deluge
may be just a new Web UI.


[0] - https://aiohttp.readthedocs.io/en/stable/

[1] - https://github.com/nathan-hoad/gbulb

--
Ticket URL: <https://dev.deluge-torrent.org/ticket/3207#comment:3>
Deluge <http://deluge-torrent.org/>
Deluge Project

[Deluge]

#3207: Core: migrate from Twisted to Asyncio
------------------------------+---------------------------
Reporter: andar | Owner: andar
Type: feature-request | Status: new
Priority: major | Milestone: 2.x
Component: Core | Version: develop (git)
Resolution: | Keywords:
------------------------------+---------------------------
Changes (by andar):

* milestone: 2.0.0 => 2.x


--
Ticket URL: <https://dev.deluge-torrent.org/ticket/3207#comment:4>
Deluge <http://deluge-torrent.org/>
Deluge Project

[Deluge]

#3207: Core: migrate from Twisted to Asyncio
------------------------------+---------------------------
Reporter: andar | Owner: andar
Type: feature-request | Status: new
Priority: major | Milestone: 2.x
Component: Core | Version: develop (git)
Resolution: | Keywords:
------------------------------+---------------------------

Comment (by JohnDoee):

This strikes me as basically a needless rewrite of Deluge.

First thing first, you can use asyncio (and all it brings with it) in
Twisted by installing the asyncio reactor. See the examples here of how
Django Channels does it here:

https://github.com/django/daphne/blob/e93643ff5a2797f05e88bc59800d7b4dbf41765d/daphne/server.py#L1-L19

https://twistedmatrix.com/documents/current/api/twisted.internet.asyncioreactor.html

The example you use to show how the code would differ tries specifically
to be obtuse as they specify keyword equivalency and then doesn't use them
equally (i.e. they skip @inlineCallbacks as that'd make the code look VERY
similar while saying async def equals it).

Using inlineCallbacks has been part of Twisted for 12-14 years, which
would make it predate Deluge? The choice of not using it everywhere must
be a design choice?

Not sure how Twisted is "overkill" in any way. Debian stripped it down
into smaller packages as it's just a small core and a large bunch of
protocols.

To end this, I'm annoyed because it'll break my own plugin while only
bringing maybe 5MB of saved space.

Feel free to change reactor but PLEASE abandon the removal of Twisted.

--
Ticket URL: <https://dev.deluge-torrent.org/ticket/3207#comment:5>
Deluge <http://deluge-torrent.org/>
Deluge Project

[Deluge]

#3207: Core: migrate from Twisted to Asyncio
------------------------------+---------------------------
Reporter: andar | Owner: andar
Type: feature-request | Status: new
Priority: major | Milestone: 2.x
Component: Core | Version: develop (git)
Resolution: | Keywords:
------------------------------+---------------------------

Comment (by andar):

I would argue that Deluge is in dire need of modernization and a rewrite
of some of the core code is warranted. Unfortunately there will be some
compatibility breaks, but they shouldn't be too difficult to fix.

The advantage really isn't in the space savings, but rather the next steps
after the move to asyncio. The use of asyncio and the new keywords makes
the code much easier to reason about and will help greatly with the
necessary refactoring and updating of the code post-migration.

I'm curious, does your plugin use Twisted extensively? Would you mind
pointing me at the code so I can have a better understanding of the burden
I'd be putting on you?

--
Ticket URL: <https://dev.deluge-torrent.org/ticket/3207#comment:6>
Deluge <http://deluge-torrent.org/>
Deluge Project

[Deluge]

#3207: Core: migrate from Twisted to Asyncio
------------------------------+---------------------------
Reporter: andar | Owner: andar
Type: feature-request | Status: new
Priority: major | Milestone: 2.x
Component: Core | Version: develop (git)
Resolution: | Keywords:
------------------------------+---------------------------

Comment (by JohnDoee):

But, as I said, you can keep using Twisted as framework while using the
asyncio functionality. This means you can use asyncio keywords, design
patterns etc. while not throwing anything made so far out the window.

https://twistedmatrix.com/documents/current/core/howto/defer-intro.html
#coroutines-with-async-await

I made the Deluge Streaming plugin and have designed a bunch around
Twisted Web to serve files (
https://github.com/JohnDoee/thomas/blob/develop/thomas/outputs/http.py ) -
The plugin itself also rely on Twisted and would require the same type of
rewrite as Deluge.

Alternatively, I can just include Twisted in my plugin but I'd prefer not
to have to do that.

--
Ticket URL: <https://dev.deluge-torrent.org/ticket/3207#comment:7>
Deluge <http://deluge-torrent.org/>
Deluge Project

[Deluge]

#3207: Core: migrate from Twisted to Asyncio
------------------------------+---------------------------
Reporter: andar | Owner: andar
Type: feature-request | Status: new
Priority: major | Milestone: 2.x
Component: Core | Version: develop (git)
Resolution: | Keywords:
------------------------------+---------------------------

Comment (by andar):

I think that maybe running Twisted on top of asyncio might make sense for
your plugin then.

https://twistedmatrix.com/documents/current/api/twisted.internet.asyncioreactor.html

{{{
import asyncio
from twisted.internet import asyncioreactor
asyncioreactor.install(asyncio.get_event_loop())
}}}

Maybe Deluge could do this for plugins so that we don't need to rewrite
them?

I understand that we could wrap all the coroutines in Deferreds and keep
using Twisted, but I really don't want to go down that route and would
much rather remove Twisted and never deal with another Deferred again.

--
Ticket URL: <https://dev.deluge-torrent.org/ticket/3207#comment:8>
Deluge <http://deluge-torrent.org/>
Deluge Project