strawberry
strawberry copied to clipboard
Add basic Channels integration
Description
This is a draft PR implementing #1408 - so far it has pretty good coverage of the websocket side of things, but I need to finish:
- the HTTP consumer
- the high level API for the Channels Layers, to make it easy to work with, especially if you're in a sync Django view.
- some more unit tests, especially in relation to the http consumer
I've just rewritten the PR to make the commits a little tidier, and also to clean up the implementation of the consumers which I wasn't happy with.
I'm currently treating it as separate from the Django integration, with its own package path in strawberry.channels - I can imagine before long all the integrations might get consolidated in a common namespace but for now I'm sticking with the same style as FastAPI, aiohttp, etc.
The most likely use case for this will be using it to provide subscriptions support on Django, with the channels layer offering a pretty nice way to integrate it with the rest of a Django project, even if it is a mostly synchronous one.
I've been building some tests, mostly re-using tests from the other GraphQL integrations (I have been wondering as well, do we really need separate test suites for each integration when it comes to websockets? Seems like something that could be done once for all of them, except perhaps for the issue that the context isn't implemented consitently?)
The Channels integration is pretty small and simple to be honest - the biggest part of this is likely to actually be a good example and some documentation (like a how-to) on how this can actually be used. I have something in progress for that, which I figure I should PR to the strawberry/examples repo...
The main thing I'm working on now that the websockets consumer implementation is fairly stable is to add the Layers high level API to make it easy to integrate with a Django app. See the issue ticket for thoughts on this.
Types of Changes
- [ ] Core
- [ ] Bugfix
- [x] New feature
- [ ] Enhancement/optimization
- [ ] Documentation
Issues Fixed or Closed by This PR
Checklist
- [x] My code follows the code style of this project.
- [x] My change requires a change to the documentation.
- [x] I have updated the documentation accordingly.
- [x] I have read the CONTRIBUTING document.
- [ ] I have added tests to cover my changes.
- [ ] I have tested the changes and verified that they work and don't break anything (as well as I can manage).
@LucidDan hey! thanks for this PR 😊 I'm going to take a look at it during the weekend :)
I've been building some tests, mostly re-using tests from the other GraphQL integrations (I have been wondering as well, do we really need separate test suites for each integration when it comes to websockets? Seems like something that could be done once for all of them, except perhaps for the issue that the context isn't implemented consitently?)
that's a good point, but I'd personally feel more confident in having one test for each integration 😊
Codecov Report
Merging #1407 (fdd6988) into main (091dd22) will increase coverage by
0.10%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## main #1407 +/- ##
==========================================
+ Coverage 98.09% 98.19% +0.10%
==========================================
Files 152 160 +8
Lines 6095 6381 +286
Branches 1160 1200 +40
==========================================
+ Hits 5979 6266 +287
Misses 58 58
+ Partials 58 57 -1
/pre-release
Pre-release
:wave:
Pre-release 0.125.0.dev.1660322955 [af4caecee0cabd279682f6d585b29390c545a105] has been released on PyPi! :rocket: You can try it by doing:
poetry add strawberry-graphql==0.125.0.dev.1660322955
I made some fixes for the HttpHandler, we might need some tests for it 😊
Thanks for adding the RELEASE.md
file!
Here's a preview of the changelog:
This release adds an integration with Django Channels. The integration will allow you to use GraphQL subscriptions via Django Channels.
Here's the preview release card for twitter:
Here's the tweet text:
🆕 Release (next) is out! This time with a new integration for Django channels. Feel free to test this out and let us know how it works!
Thanks to @Lucid_Dan, @_bellini666 and Nir (https://github.com/nrbnlulu) for working on this PR!
Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)
Hey guys. I've been working on this PR on the past week and I think it should be ready to be reviewed.
I took over from where @LucidDan with the following changes:
-
I integrated the code with the current state of strawberry, integrating some features like query execution using GET parameters, etc
-
The
HttpConsumer
is finished and covered with tests -
I added a high level api to consume events from layers in a way similar to what @LucidDan proposed at https://github.com/strawberry-graphql/strawberry/issues/1408. That is also covered with tests
Will be ready to do any changes necessary for us to merge this as soon as possible :)
Thanks!
Curious as too when this will be merged into a release. I am wanted to try and give it a spin as I have project that I would like to use it in.
hi @Xorso, I'll do a pre-release for this so you can test it out :) I'll try to do a review soon :) I was a bit too busy up until a few days ago :)
/pre-release
This can be tested with poetry add strawberry-graphql==0.115.0.dev.1655415925
:)
Just thought I would bring this up as I am testing. It appears that when I use the GraphQLProtocolTypeRouter
in my asgi.py
I get a 500 error while trying to access the graphiql page in the browser. Just wondering if I am doing something wrong.
from daziadmin.backend.schema import schema
application = GraphQLProtocolTypeRouter(
schema,
django_application=django_asgi_app
)
Runserver logs
System check identified no issues (0 silenced).
June 21, 2022 - 21:29:44
Django version 4.0.4, using settings 'config.settings.local'
Starting ASGI/Channels version 3.0.4 development server at http://0.0.0.0:8000/
Quit the server with CONTROL-C.
INFO 2022-06-21 21:30:26,571 server 2623180 140310638642944 HTTP/2 support not enabled (install the http2 and tls Twisted extras)
INFO 2022-06-21 21:30:26,572 server 2623180 140310638642944 Configuring endpoint tcp:port=8000:interface=0.0.0.0
INFO 2022-06-21 21:30:26,576 server 2623180 140310638642944 Listening on TCP address 0.0.0.0:8000
HTTP GET /graphql 500 [0.01, 127.0.0.1:46024]
ERROR 2022-06-21 21:30:31,777 runserver 2623180 140310638642944 HTTP GET /graphql 500 [0.01, 127.0.0.1:46024]
EDIT:
May have found where the problem exists. No solution yet. http_handler.py
def should_render_graphiql(self):
return self.graphiql and self.headers.get("accept", "") in ["text/html", "*/*"]
This seems to return false. Specifically the self.headers.get in
:
self.headers.get("accept", "") in ["text/html", "*/*"]
False
self.headers.get("accept", "")
'text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9'
If I force it to be true it simply renders the page with carriage returns.
Hey @Xorso , I'll take a look at this probably tomorrow and send a fix. From what I can see the should_render_graphiql
method is checking the accept header in the wrong way.
Thank you. Also, I haven't been able to get grahipql to come up either even when i set it to true. Again not sure if I am doing something wrong there.
@Xorso I just installed the pre-release version on a project of mine and set it up using both the websocket consumer and the http consumer. I could replicate your issues and fixed both of them. From my tests, that was the only issue that I found.
@patrick91 can we get another pre-release?
/pre-release
this is the last version: poetry add strawberry-graphql==0.115.0.dev.1655918067
So far so good. Things are looking better. Thanks for the quick turn around. I will keep implementing it and let you know the results.
Wondering if we can add some additional helper methods for cookies, headers, user etc. to ChannelsConsumer
for more interoperability?
Something like
@property
def headers(self) -> Dict[str, str]:
return {
header_name.decode().lower(): header_value.decode()
for header_name, header_value in self.scope["headers"]
}
@property
def cookies(self) -> Dict[str, str]:
return {
cookie_name.decode().lower(): cookie_value.decode()
for cookie_name, cookie_value in self.scope["cookies"]
}
@property
def META(self):
meta = {}
for name, value in self.scope.get("headers", []):
name = name.decode("latin1")
if name == "content-length":
corrected_name = "CONTENT_LENGTH"
elif name == "content-type":
corrected_name = "CONTENT_TYPE"
else:
corrected_name = "HTTP_%s" % name.upper().replace("-", "_")
# HTTPbis say only ASCII chars are allowed in headers, but we
# use latin1 just in case
value = value.decode("latin1")
if corrected_name in meta:
value = meta[corrected_name] + "," + value
meta[corrected_name] = value
return meta
@property
def COOKIES(self):
return self.cookies
@property
def user(self):
return self.scope["user"]
@user.setter
def user(self, value):
self.scope["user"] = value
Maybe this is too specific to Django, and needs to be elsewhere.
Hey @jaydensmith . IMO I think that it is indeed to specific to Django, but you should be able to easily define your own custom consumer, inheriting from strawberry's one, and extend it with that. I actually do that for django itself in my projects, overriding the strawberry's view to use my custom context instead of the default one.
/pre-release
Hi 👋 You can find a preview of the docs here:
https://strawberry.rocks/docs/pr/1407/integrations/channels https://strawberry.rocks/docs/pr/1407/README https://strawberry.rocks/docs/pr/1407/general/subscriptions https://strawberry.rocks/docs/pr/1407/integrations/django
I was wondering if it would be possible / useful to map event types to strawberry subscriptions. i.e
@strawberry.type
class Mutation:
@strawberry.mutation
async def send_chat_message(
self,
room: ChatRoom,
message: str,
) -> None:
channel_layer = get_channel_layer()
await channel_layer.group_send(
f"chat_{room.room_name}",
{
"type": Subscription.join_chat_rooms, # <<<
"room_id": room.room_name,
"message": message,
},
)
@strawberry.type
class Subscription:
@strawberry.subscription
async def join_chat_rooms(
self,
info: Info,
rooms: List[ChatRoom],
user: str,
) -> AsyncGenerator[ChatRoomMessage, None]:
"""Join and subscribe to message sent to the given rooms."""
ws = info.context.ws
async for message in ws.channel_listen(groups=rooms): # <<<
yield ChatRoomMessage(
room_name=message["room_id"], message=message["message"], current_user=user
)
The subscriptions has been working great! And it looks like the docs have come along as well. I would actually like to take this to production but need to have an official release for it. Do you know if this is scheduled to be merged? Or is it still up in the air?
Thank you again for your work on this.
The subscriptions has been working great! And it looks like the docs have come along as well. I would actually like to take this to production but need to have an official release for it. Do you know if this is scheduled to be merged? Or is it still up in the air?
Thank you again for your work on this.
Hi @Xorso I'll do by best to review and get this merged in the next few days 😊
The subscriptions has been working great! And it looks like the docs have come along as well. I would actually like to take this to production but need to have an official release for it. Do you know if this is scheduled to be merged? Or is it still up in the air? Thank you again for your work on this.
Hi @Xorso I'll do by best to review and get this merged in the next few days blush
@patrick91 Could you keep in mind my while reviewing that my pr #2046 should replace the docs over here, I added there a tutorial continuing from the channels chat tutorial, some testing guides and minor API reference.
The subscriptions has been working great! And it looks like the docs have come along as well. I would actually like to take this to production but need to have an official release for it. Do you know if this is scheduled to be merged? Or is it still up in the air? Thank you again for your work on this.
Hi @Xorso I'll do by best to review and get this merged in the next few days blush
@patrick91 Could you keep in mind my while reviewing that my pr #2046 should replace the docs over here, I added there a tutorial continuing from the channels chat tutorial, some testing guides and minor API reference.
@nrbnlulu yes, if you're ok I can merge your PR here 😊
@patrick91 Sure!
Looks like this is getting closer. Anything I can do to help?