Initial Quart Subscription Support #3818
Conversation
Reviewer's Guide by SourceryThis pull request introduces initial support for GraphQL subscriptions in Strawberry running on Quart. It includes a WebSocket adapter, updates the GraphQL view to handle WebSocket requests, and adds necessary testing infrastructure. Updated class diagram for GraphQLViewclassDiagram
class GraphQLView {
-schema: Schema
-allow_queries_via_get: bool
-keep_alive: bool
-keep_alive_interval: float
-debug: bool
-subscription_protocols: list[str]
-connection_init_wait_timeout: timedelta
-multipart_uploads_enabled: bool
+__init__(
schema: Schema,
graphiql: Optional[bool] = None,
graphql_ide: Optional[GraphQL_IDE] = "graphiql",
allow_queries_via_get: bool = True,
keep_alive: bool = True,
keep_alive_interval: float = 1,
debug: bool = False,
subscription_protocols: list[str] = [
GRAPHQL_TRANSPORT_WS_PROTOCOL,
GRAPHQL_WS_PROTOCOL,
],
connection_init_wait_timeout: timedelta = timedelta(minutes=1),
multipart_uploads_enabled: bool = False,
) -> None
+is_websocket_request(request: Request) TypeGuard[Request]
+pick_websocket_subprotocol(request: Request) Optional[str]
+create_websocket_response(request: Request, subprotocol: Optional[str]) Response
+register_route(cls: Quart, rule_name: str, path: str, **kwargs)
}
class AsyncBaseHTTPView
GraphQLView --|> AsyncBaseHTTPView : inherits
note for GraphQLView "Adds websocket support"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Some manual testing indicates that things work as I'd expect them. But as of right now, I'm still struggling to figure out the issues with the automatic testing. It appears that the Quart Test Client tries to be a bit too helpful by throwing exceptions for many of the cases where we'd want to get the original message: After struggling with it for hours, I've decided to try the Starlette Test Client and simply treat the Quart application as any old ASGI application. With that I managed to make some progress, but again Quart seems to want some specific ASGI spec version to pass on the full connection close message: https://github.com/pallets/quart/blob/main/src/quart/asgi.py#L347-L348 Any ideas what else I could try here? |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
I've managed to make the Quart test client work, and now we're at just 20 Test failures. However, I'm not quite sure how to progress on those. Some are simply a limitation of the test client, like not being able to tell what sub protocol was selected, others seem like I'd need to understand the quart test app implementation even more to really figure them out. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3818 +/- ##
==========================================
- Coverage 97.23% 95.22% -2.02%
==========================================
Files 503 499 -4
Lines 33580 32571 -1009
Branches 1717 1703 -14
==========================================
- Hits 32653 31016 -1637
- Misses 708 1283 +575
- Partials 219 272 +53 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #3818 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Hi @treo, thanks for the PR!
I left some comments just so that I won't forget about them later. Let's first focus on getting the test to pass and then polish everything at the end.
As for the tests, I liked your idea on Discord to use the starlette test client to work around the issues the quart test client is causing. Since quart is an ASGI framework, I don't see a reason against using the startlette ASGI test client, especially since this is our internal test client only used for testing.
| @classmethod | ||
| def register_route(cls, app: Quart, rule_name: str, path: str, **kwargs): | ||
| """Helper method to register both HTTP and WebSocket handlers for a given path. | ||
|
|
||
| Args: | ||
| app: The Quart application | ||
| rule_name: The name of the rule | ||
| path: The path to register the handlers for | ||
| **kwargs: Parameters to pass to the GraphQLView constructor | ||
| """ | ||
| # Register both HTTP and WebSocket handler at the same path | ||
| view_func = cls.as_view(rule_name, **kwargs) | ||
| app.add_url_rule(path, view_func=view_func, methods=["GET", "POST"]) | ||
|
|
||
| # Register the WebSocket handler using the same view function | ||
| # Quart will handle routing based on the WebSocket upgrade header | ||
| app.add_url_rule(path, view_func=view_func, methods=["GET"], websocket=True) |
There was a problem hiding this comment.
I think I'd prefer if we left it up to the user to register the two URL rules. The examples in our docs for adding HTTP/WS/HTTP+WS URL rules would then end up looking more consistent.
There was a problem hiding this comment.
This is more of a convenience method, but if you think that just documenting it is good enough, then I'm fine to remove it. Code that doesn't exist doesn't have any bugs :D
| async def close(self, code: int, reason: str) -> None: | ||
| await self.ws.close(code, reason=reason) | ||
|
|
||
|
|
||
| class GraphQLView( | ||
| AsyncBaseHTTPView[ | ||
| Request, Response, Response, Request, Response, Context, RootValue |
There was a problem hiding this comment.
Ideally, we would adjust these types to include quart.Websocket and make create_websocket_response return websocket etc. so that we end up with sound types.
That being said, I recognize quart loves their global context vars, so we could go with None here too. In that case get_context might need an update, since it's currently just using the return value from create_websocket_response which is None.
There was a problem hiding this comment.
Yeah, I wasn't quite sure how to approach this exactly because of the context vars, But once the tests are all passing, we can take a look at cleaning this up.
for more information, see https://pre-commit.ci
Description
Strawberry currently doesn't support subscriptions on Quart. This PR aims to rectify that.
Types of Changes
Checklist
Summary by Sourcery
Adds initial support for GraphQL subscriptions using Quart, including WebSocket handling and integration with the strawberry library. It also includes necessary updates to the test suite to ensure the new functionality works as expected.
New Features:
Tests: