New module to support Django Channels v2 #18
Conversation
|
@SmileyChris this looks good to me |
Also add the (unexpectedly missing) examples to the distribution
Fixes future operation requests that were being blocked
Promise observers are already futures, the only thing that needs to be a future is the on_message call from the receive_json Django consumer
|
@SmileyChris do you have any recommendation for running behind nginx in production? I tested this PR out and it worked out well for me. I believe i have i have figured out a configuration to get on to production with gunicorn/uvicorn/nginx |
|
@syrusakbary Is there anything which I can do to make it easier for you to feel confident in including this code? |
SmileyChris
force-pushed
the
channels2
branch
from
6a3ca47 to
d8d83ca
Compare
|
Any news on when we can get this merged? The check fails because of missing |
|
I fork this https://pypi.org/project/graphql-ws-django/ and does not work with docker |
|
I checked your fork which seems promising. Since this is not yet merged, I tried to install your branch by pipenv / pip, yet result in WinErrors(directories not found). Is it installable? I hate to use the code by copying the code into my own project :) @Manviel Is your pypi release based on this fork? I can just install from there? Update: I installed from @Manviel pypi release however, testing via /graphiql/, the websocket keep getting bad handshake (500) and disconnect repeatedly. Not sure how to debug since there isn't any info related outputted to my PyCharm console. INFO WebSocket HANDSHAKING /subscriptions [127.0.0.1:63069]
INFO WebSocket DISCONNECT /subscriptions [127.0.0.1:63069]
INFO WebSocket HANDSHAKING /subscriptions [127.0.0.1:63078]
INFO WebSocket DISCONNECT /subscriptions [127.0.0.1:63078]Also, from channels.generic.websockets import JsonWebsocketConsumershould be from channels.generic.websocket import JsonWebsocketConsumer |
|
@gotexis Well I'm using my branch in a pre-production project just fine (granted, not on Windows). |
My development is on Windows T T |
|
@Cito Thank you for your comment. I just spend ~4h getting this working (PyPI is out of date, the doc is all over the place) and @SmileyChris 's code is the next best thing for anyone using graphene Django today.
I understand your concern and from my point of view, that is what Chris did. He moved the bucket forward for us towards having a more complete coverage and, so far, his code is "up-to-date" and working (if you do not use the outdated PyPI). To you point: who knows what the future is made of? Final thought, I do not know if you realize it but this repro is listed on the graphene python website as the gold standard: https://docs.graphene-python.org/projects/django/en/latest/subscriptions/ Please help us by approving this PR: "A journey of a thousand miles begins with a single step" :-) Thank you!! |
| Gevent | ||
| ~~~~~~ | ||
| GRAPHENE = { | ||
| 'SCHEMA': 'yourproject.schema.schema' |
There was a problem hiding this comment.
this tripped me up: you should mention in the doc that it is required for the channel work to function where it is technically not needed for graphene to work.
I had to add to my existing project.
There was a problem hiding this comment.
This is under the django channels configuration section, so it would imply that it's the setup required for channels, wouldn't it? Maybe I'm not understanding - please elaborate
There was a problem hiding this comment.
This is a setup I found on the Django-graphene website: https://docs.graphene-python.org/projects/django/en/latest/tutorial-plain/#update-settings
It is "new"ish and, so far, works without it unless you activate the subscriptions (not sure why).
I point it out for anyone like me who has a working project setup before the django-graphene update that works as-is: subscriptions will not work and the error is not that clear. To be clear, I am using the latest stable version of the django-graphene though. mystery mystery
It is a suggestion because I chased my tail for ~20mins looking at an opaque error ;-)
| "default": { | ||
| "BACKEND": "asgiref.inmemory.ChannelLayer", | ||
| "ROUTING": "django_subscriptions.urls.channel_routing", | ||
| }, |
There was a problem hiding this comment.
the example is out-dated. They retired this. I used:
"default": {"BACKEND": "channels.layers.InMemoryChannelLayer",},
}
instead, and that works now
There was a problem hiding this comment.
This is in the outdated "old django" section anyway, is that backend configuration really retired in channels v1?
There was a problem hiding this comment.
From what I found asgiref retired their in-memory channel layer. So, if you get asgiref, it no longer has the class listed in the doc :-/
about the ROUTING part, idk, I only tested channels V2 and having that key with V2 was crashing (actively rejected, not ignored).
There was a problem hiding this comment.
Sorry, I don't have the time to review this. Looks you're making good progress, so approving this to no block you. But as I mentioned, please discuss on the Graphene Slack how this project can be managed, who can become responsible for this project and do the reviews and approvals in the future. As much as I would like to help, I really don't have the time and would like to pass this off to somebody else.
| Gevent | ||
| ~~~~~~ | ||
| GRAPHENE = { | ||
| 'SCHEMA': 'yourproject.schema.schema' |
There was a problem hiding this comment.
This is under the django channels configuration section, so it would imply that it's the setup required for channels, wouldn't it? Maybe I'm not understanding - please elaborate
…plicitly returned
Rather than modify the existing channels v1 implementation, this PR adds in a separate package (
graphql.django) to handle channels v2.It leverages some of the implementation of other modules (there is a bunch of similar looking code that probably needs to be refactored into a common base class really, but that's outside of scope).
The example should work out of the box from within the
examples/django_channels2/path with apipenv install,./manage.py runserver