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

RFC: Should discord.py be asyncio compatible? #50

Closed
Rapptz opened this Issue Nov 30, 2015 · 9 comments

Comments

Projects
None yet
7 participants
@Rapptz
Owner

Rapptz commented Nov 30, 2015

One of the biggest problems people usually talk about with the library is how the library is inherently blocking, either through the websocket itself or the events themselves. These issues are not inherently related to the library being async or not but a consequence of not having a threading model set up for these things.

That aside, an often mentioned thing people want from the library is for it to be more async. I won't divulge on the fact whether those people actually know the pros and cons but I will list them below. I hope to use this issue as a RFC to see if it's truly worth considering.

These lists are non-exhaustive.

Cons

  • The API will be somewhat harder to understand for newcomers and veterans alike.
    • A lot of the methods will become coroutines and a result you need to do something like yield from (or await in Python 3.5+) to actually invoke it.
      • This is quite a pitfall if you forget it and a common source of asyncio errors.
    • Another pitfall would be forgetting to create a function to invoke the library listeners itself in the asyncio event loop.
  • The library will have to be essentially rewritten to an extent.
    • Only discord.Client is heavily affected since the data classes just hold and parse data.
    • discord.Client is a large chunk of the library and rewriting is a very annoying task.
    • Essentially, time spent rewriting the library means time spent less on other features.
  • Support for Python versions lower than 3.4 will be dropped.
    • Python version 3.3 is supported via the backport of asyncio module found in pip.
  • This will be the biggest breaking change that the library has ever had.
    • All previous code will definitely not work.

Pros

  • The library will obviously become asynchronous (should you take advantage of it).
  • Rewriting the discord.Client will allow me to clean-up the code significantly.
  • Dropping support for Python 2.7 will alleviate personal issues with handling compatibility with fundamental differences.
    • bytes vs str vs unicode is one of the biggest compatibility hurdles that I put up with and removing it is a good thing.

Why asyncio? Why not Twister or gevent or my favourite library here?

asyncio comes built-in and is the future of asynchronous programming in Python.
The language itself now comes with full support for the stuff in asyncio module such as coroutines.
You can write async def blocks, async for loops, and async with statements. The support for
asynchronous programming in Python is essentially all based on the core concepts made in asyncio.

Another pro is that I can use libraries that are developed with asyncio in mind such as aiohttp
and have little external dependencies on the user.

Performance of these libraries is irrelevant. Python as a language is already "slow" and seeking
a lot of bleeding edge speed for the async library will probably not outweigh the benefits of
having to use a standardised library.

I am willing to reconsider other options but for now, I am strictly approaching this design by only
considering asyncio and nothing else.

Conclusion

The purpose of this RFC is to gauge library user interests and asking if these changes are worth it
going forward. If you have anything to add please comment below. I'll be reading all the comments
here and taking them seriously.

You can also use this thread to vote on the issue.

Of course, here's a link to the library if you're curious of how it is and the Discord API server where discussion will also take place.

@AraHaan

This comment has been minimized.

AraHaan commented Nov 30, 2015

tbh I would like the lib to be as easy as it could be to use but at same time it needs to be as flaw free as possible as well as we need Voice Channel to be developed more.

@CarlosFdez

This comment has been minimized.

CarlosFdez commented Nov 30, 2015

As far as specific pros and cons go, I think the difficulty for newcomers is more important than it appears. Few problems bait people to learn programming like bots do, and for better or worse, beginners are drawn to Python. However, while it may also be more difficult for veterans as well, I'm sure that once asyncio becomes more standard better learning materials will begin to appear.

While I do want asyncio, I want it because it is a solution to the actual problem: handling processes that run parallel to the client. Currently, threads have to be spawned manually, and I am not sure which methods on the client are threadsafe. Currently, I have to check the source code to ascertain that, and even then I'm not sure if the underlying websocket itself is threadsafe. While it is fine to use most methods that use requests, having to examine the source code to know what I can or can't do with the library stinks.

I am not familiar enough with Python yet to suggest alternatives, but if an implementation that uses threading or some other means solves the same problem but with less fuss (read: less cons), I'd be all for it. I've had a few thoughts but none of them seem good.

@Red-M

This comment has been minimized.

Contributor

Red-M commented Nov 30, 2015

The requests lib is thread safe, the issue you'll have for threading is the underlying websocket being passed correctly and if you take this route it should be fine as long as you keep the main thread for discord.py free, HOWEVER, you will need to make sure that discord.py in its entirety is thread safe if you choose to multi-thread or to multi-process.

You may want to take a look at the multiprocess module for python as it may solve some issues with being thread safe and also keep the main loop of the program block free and in some respects you could just say that the client.login should be multiprocess'ed if the program needs to not be blocked for other tasks (aka, its not a problem in the scope of this library).

I'm fine with moving to asyncio but be careful with python2 support if you go this way as a lot of new comers will try python2 first (trust me I know this happens a lot with a similar project that switched to python3 and asyncio)

Also be aware that python 3.5 may not be packaged in some linux distros so that's also something to consider.

The final aspect is the time aspect, do you and the other contributors have the time to rewrite from the ground up?
This will most likely take a long time to do in comparison to the first write as you will need to match the old structure with the rewrite.

Anyway, I'm fine with how the library is right now but I'm ok if you want to do asyncio or any of the options I have presented.

@Hornwitser

This comment has been minimized.

Contributor

Hornwitser commented Nov 30, 2015

My use case is rather unique in that I spawn up tens of Discord.py bots at the same time, and in large channels my IRC bridge would spawn hundreds. Given the weak control of threads in python, it's easy to end up with zombie bots when using a thread per bot, as I'm currently doing (and having issues with). I know how I can use ws4py.manager in the current implementation to use a single thread to drive multiple bots and gain more control, but I have no idea whether an asyncio implementation would be equally easy to adapt for multiple bots on one thread.

The most common reason I've seen for requesting the library to be more asynchronous has been to make it possible to write event handler that does long operations in them without blocking the rest of the client, a thread safe way to act on events in threads outside of the library, and better support of delayed actions. The first can only be reasonably solved using a thread pool, and even there there's issues with starvation and deadlocks that could occur. The second is already supported using Client.dispatch and custom events (though it's not documented, and therefore very little known), and last one would be the only one really benefiting from a move to the async library.

That is the two things I have to say on this issue, I myself is fine with either case of rewriting or not.

@anowlcalledjosh

This comment has been minimized.

anowlcalledjosh commented Nov 30, 2015

Python 3.5 is not currently (AFAIK) available in the Ubuntu repos, at least not for 14.04. Once 16.04 drops (hopefully with 3.5 as the only Python preinstalled) the situation should improve somewhat, but at the moment I don't think only supporting 3.4/3.5 is a good move, especially if full support for asyncio was only added in 3.5.

@anowlcalledjosh

This comment has been minimized.

anowlcalledjosh commented Nov 30, 2015

OTOH, Python 2.x support should IMHO be dropped as soon as possible, i.e. now. There's no good reason to use it for new projects, and given discord.py has only existed since Python 3 there's no reason it should be backwards-compatible. Python 2 also actively confuses newcomers to the language, and we should presumably be encouraging those learning Python in order to make a bot to learn the new, sane version that copes with Unicode properly rather than the old, bug-fixes-only version.

@Rapptz

This comment has been minimized.

Owner

Rapptz commented Nov 30, 2015

I don't know where the notion that asyncio support was only added in 3.5 comes from.

The asyncio package is added in 3.4.0 and mostly solidified in 3.4.3. Python 3.5 gives asyncio leverage in the language by having language specific constructs but it is backwards compatible with the coroutines and asynchronous aspects of python 3.4. Ubuntu LS 14.04 which has the crappiest package updating in all linux distros I know of ships with 3.4.3 so it should not be a problem.

@gdraynz

This comment has been minimized.

gdraynz commented Dec 17, 2015

Definitely should, it smoothes a lot any library and is largely mature enough to be used without problem.
Python 3.5 will also be the default python version in ubuntu in the next LTS version, and the other distributions will probably follow.

@Rapptz Rapptz modified the milestone: v0.10.0 Dec 17, 2015

@Rapptz

This comment has been minimized.

Owner

Rapptz commented Dec 30, 2015

I'm going to close this since development on asyncio compat has already started in the async branch and will be merged into master when v0.10.0 launches. The branch is located in https://github.com/Rapptz/discord.py/tree/async

@Rapptz Rapptz closed this Dec 30, 2015

Repository owner locked and limited conversation to collaborators Jun 12, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.