-
Notifications
You must be signed in to change notification settings - Fork 168
feat: support batch queries #550
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
base: main
Are you sure you want to change the base?
feat: support batch queries #550
Conversation
graphql_client/src/reqwest.rs
Outdated
client: &reqwest::blocking::Client, | ||
url: U, | ||
variables: Vec<Q::Variables>, | ||
) -> Result<crate::Response<Q::ResponseData>, reqwest::Error> { |
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.
Shouldn't this be Result<Vec<crate::Response<...>>, _>
?
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.
it will be serde_json::Array
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.
I think the shape of the ResponseData is from generated code, not a serde_json::Value, so I don't think serde_json::Array would appear anywhere, right?
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.
Also, this approach only works if it's a batch of the same query — in GraphQL, the queries can be different. It would not be straightforward to statically type, though.
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.
@tomhoule, If we want different queries, we should serialize the queries and have a serde_json::Value as a result since the trait GraphqlQuery is not object safe. if that's okay with you I can do that.
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.
There's little utility to that, since this crate is only about the type safety. Let's just document that it's a batch of identical queries.
My only reservation left is that I still don't understand why this doesn't return Result<Vec<crate::Response<Q::ResponseData>>, reqwest::Error>
instead of Result<crate::Response<Q::ResponseData>, reqwest::Error>
.
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.
yes it makes more sense to have it as Vec<Q::ResponseData>, I did update it
@tomhoule could you please launch the tests again |
Adds support for executing multiple GraphQL operations in a single HTTP request (batch query).
This enables sending an array of query objects with their respective variables, improving performance for use cases that require multiple small requests.
Includes:
Support for serializing and sending batched query payloads.
Adjusted request builder to handle both single and batch query modes.
Unit tests covering multi-query payload serialization and response handling.