-
-
Notifications
You must be signed in to change notification settings - Fork 508
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 extensions to the HTTP protocol #3461
base: main
Are you sure you want to change the base?
Conversation
Hi, thanks for contributing to Strawberry 🍓! We noticed that this PR is missing a So as soon as this PR is merged, a release will be made 🚀. Here's an example of Release type: patch
Description of the changes, ideally with some examples, if adding a new feature. Release type can be one of patch, minor or major. We use semver, so make sure to pick the appropriate type. If in doubt feel free to ask :) Here's the tweet text:
|
1 similar comment
Hi, thanks for contributing to Strawberry 🍓! We noticed that this PR is missing a So as soon as this PR is merged, a release will be made 🚀. Here's an example of Release type: patch
Description of the changes, ideally with some examples, if adding a new feature. Release type can be one of patch, minor or major. We use semver, so make sure to pick the appropriate type. If in doubt feel free to ask :) Here's the tweet text:
|
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.
Hey @omarzouk - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
@@ -33,6 +33,7 @@ class GraphQLRequestData: | |||
query: Optional[str] | |||
variables: Optional[Dict[str, Any]] | |||
operation_name: Optional[str] | |||
extensions: Optional[Dict[str, Any]] |
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.
suggestion (code_clarification): Consider renaming 'extensions' to 'protocol_extensions' for consistency.
Aligning the naming of 'extensions' to 'protocol_extensions' as used in other parts of the codebase enhances readability and maintainability.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3461 +/- ##
=======================================
Coverage 96.49% 96.49%
=======================================
Files 509 509
Lines 32862 32864 +2
Branches 5421 5421
=======================================
+ Hits 31710 31713 +3
+ Misses 941 940 -1
Partials 211 211 |
CodSpeed Performance ReportMerging #3461 will not alter performanceComparing Summary
|
Description
The GQL spec specifies a field called extensions as part of the request parameters spec in the HTTP protocol. We are missing support for this
Adding the field to be parsed and propagating it as part of the
ExecutionContext
so applications can access itTypes of Changes
Issues Fixed or Closed by This PR
extensions
Request Parameter for the HTTP protocol #3224Checklist