graphql-python-subscriptions icon indicating copy to clipboard operation
graphql-python-subscriptions copied to clipboard

Refactor code base to add ability for multiple concurrency models / executors

Open hballard opened this issue 8 years ago • 6 comments
trafficstars

Initial executors will include gevent, asyncio, and django channels

hballard avatar Jun 25 '17 04:06 hballard

Well done! Do you need some help for the django-channels executor?

Thibaut-Fatus avatar Jul 27 '17 12:07 Thibaut-Fatus

We've been looking into using django-channels as executor, and even if the use of channels as WS transport medium seemed quite straightforward, replicating your threading / asynchrone code for channels feels weird since they already use a event queue and thus are non blocking, Any thoughts on this ?

Thibaut-Fatus avatar Aug 02 '17 08:08 Thibaut-Fatus

Can you give me a specific example? I'm not super familiar w/ django channels...so that may very well be. I was going to have to play around w/ it once I got to that point to see what was possibly. But, if you can give me an example of the specific threading / async code you are referring to, then I'd be happy to offer any advice I have. If you are talking about the keep_alive_timer function or the background task that checks the redis-pubsub for messages, then I was thinking to try and use a custom django consumer (with maybe an os thread?).

Honestly, it might be that we need to have separate subscription manager, pubsub or server classes, beyond just a new executor class (maybe a mixin of some sort on top of a base class). I'm actually struggling w/ the asyncio integration a bit right now in trying to get the subscription server tests to pass, using just a new executor. The need to have async def coroutines in order to be able to await / yield from a code block really makes it hard to not duplicate code logic when integrating asyncio...which I was really not wanting to do. In some case I can get away with just scheduling a coroutine into a task using asyncio.ensure_future() at the very end of a function, but in other scenarios the control flow really demands that I'm able to await that block of code. I suppose I could also attach a callback to the returned future, but again, it means duplicating code.

Also, did you happen to see this post from syrusakbary over the weekend. This would take care of the subscription manager and pubsub classes I suppose, but there would still need to be separate integration / plugin for each async framework integrating the websocket / concurrency model (django, gevent, asyncio, etc.).

hballard avatar Aug 02 '17 13:08 hballard

Seems like he will integrate django-channels : 'websockets with django-daphne'

In your implementation, in order to achieve non-blocking code, you use asyncio or gevent, but django-channels have their own PubSub mechanism and thus I think it would be quite different from your architecture. In fact using Channels we have no access to the underlying ws, the one needed as argument when you create your SubscriptionServer

Thibaut-Fatus avatar Aug 02 '17 13:08 Thibaut-Fatus

I just re-read his post and I agree...sounds like he will integrate them. I was still playing around w/ this asyncio refactor, because I had already started it and it gave me a chance to explore asyncio (which I had really not done before), but after seeing that post this weekend, I've been thinking I would just wait for graphene 2.0 and maybe see if I can contribute to graphql-core or graphene, instead of continuing to evolve this implementation.

hballard avatar Aug 02 '17 14:08 hballard

Here's a gist with my solution using django-channels. It's a working proof of concept. Next task is optimize it for production environment. Feedback welcome!

tricoder42 avatar Jan 18 '18 08:01 tricoder42