-
-
Notifications
You must be signed in to change notification settings - Fork 533
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
Add support for multipart subscriptions #3076
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3076 +/- ##
==========================================
- Coverage 96.82% 96.79% -0.04%
==========================================
Files 514 517 +3
Lines 33322 33528 +206
Branches 5528 5575 +47
==========================================
+ Hits 32263 32452 +189
- Misses 835 847 +12
- Partials 224 229 +5 |
CodSpeed Performance ReportMerging #3076 will degrade performances by 28.11%Comparing Summary
Benchmarks breakdown
|
9902c9d
to
93e4041
Compare
for more information, see https://pre-commit.ci
Hi 👋 You can find a preview of the docs here: |
~~one thing I'm not sure about this is handling client closing the connections 🤔 ~~ edit: this seems to be handled by fastapi, I need to check django and others, also I was using next.js' proxy for this, and that was preventing the disconnect message from being sent |
fc8d0d8
to
7ed9e0e
Compare
b647e7d
to
f0cffb1
Compare
for more information, see https://pre-commit.ci
ae702e0
to
5e7c136
Compare
🔥 |
return await subscribe( # type: ignore | ||
schema, | ||
execution_context.graphql_document, | ||
root_value=execution_context.root_value, | ||
context_value=execution_context.context, | ||
variable_values=execution_context.variables, | ||
operation_name=execution_context.operation_name, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This extends the Schema.execute
method to handle subscriptions. Im wondering whether this was intentional since we already have a separate Schema.subscribe
method? 👀 @patrick91 @bellini666
I noticed this change while upgrading to the latest version. In my codebase mypy now complains everywhere that Schema.execute
can now return both ExecutionResult
and SubscriptionExecutionResult
. Before Schema.execute
could only return an ExecutionResult
while subscriptions were handled by Schema.subscribe
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that was poor design on my part, it will be fixed in #3554 😊
This is a new protocol created by Apollo (and supported by Apollo Client)
TODO
{}
is enough)Protocol docs: https://www.apollographql.com/docs/router/executing-operations/subscription-multipart-protocol/