-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[GraphQL] Add a separate timeout field in ServiceConfig for mutations #17742
[GraphQL] Add a separate timeout field in ServiceConfig for mutations #17742
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@stefan-mysten is attempting to deploy a commit to the Mysten Labs Team on Vercel. A member of the Team first needs to authorize it. |
@@ -42,6 +44,13 @@ impl Extension for TimeoutExt { | |||
next: NextParseQuery<'_>, | |||
) -> ServerResult<ExecutableDocument> { | |||
let document = next.run(ctx, query, variables).await?; | |||
let is_mutation = document |
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.
another noob q: parse_query and validation work on the entire request document right? Wouldn't the logic below set any query to is_mutation = True
as long as one of the top-level operations is a mutation?
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.
Great question which made me think again about this. At least in the IDE, you cannot run both a query request and a mutation at the same time, so for mutations you have to specify mutation {executeTransactionBlock(args) {fields}}
, so this would return True, and for a query { chainIdentifier }
or just { chainIdentifier }
would return false because the OperationType
is Query
.
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.
Nice! Left some comments on implementation details, but the core logic here is all good, thanks @stefan-mysten.
Deployment failed with the following error:
|
fe0a0cd
to
c103118
Compare
16b5de4
to
fb661bb
Compare
…MystenLabs#17742) ## Description Increases the timeout for mutation requests. ## Test plan Updated existing tests to also include this `mutation` timeout case. ``` cargo nextest run --features pg_integration -- tests::test_timeout ``` ![image](https://github.com/MystenLabs/sui/assets/135084671/3e9b9d2d-dac2-4eef-8976-f012d6bc44d9) --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [x] GraphQL: Increased the `timeout` value for when executing a transaction block. - [ ] CLI: - [ ] Rust SDK:
…MystenLabs#17742) ## Description Increases the timeout for mutation requests. ## Test plan Updated existing tests to also include this `mutation` timeout case. ``` cargo nextest run --features pg_integration -- tests::test_timeout ``` ![image](https://github.com/MystenLabs/sui/assets/135084671/3e9b9d2d-dac2-4eef-8976-f012d6bc44d9) --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [x] GraphQL: Increased the `timeout` value for when executing a transaction block. - [ ] CLI: - [ ] Rust SDK:
…MystenLabs#17742) ## Description Increases the timeout for mutation requests. ## Test plan Updated existing tests to also include this `mutation` timeout case. ``` cargo nextest run --features pg_integration -- tests::test_timeout ``` ![image](https://github.com/MystenLabs/sui/assets/135084671/3e9b9d2d-dac2-4eef-8976-f012d6bc44d9) --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [x] GraphQL: Increased the `timeout` value for when executing a transaction block. - [ ] CLI: - [ ] Rust SDK:
Description
Increases the timeout for mutation requests.
Test plan
Updated existing tests to also include this
mutation
timeout case.Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.
timeout
value for when executing a transaction block.