feat: schema delete retries#8230
Conversation
There was a problem hiding this comment.
The noise here is mostly indent changing. Relevant changes start at L1670
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for retrying schema deletion when the registry is busy or locked. Feedback highlights several critical issues: a GraphQL syntax error where False is capitalized instead of lowercase false, a resolver bug where the supportsRetry flag is not forwarded to the publisher, a tight loop in the CLI retry logic that lacks a delay, and an inaccurate retry reason string referencing 'publish' instead of 'operation' during a delete.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (result.schemaDelete.__typename === 'SchemaDeleteRetry') { | ||
| this.log(result.schemaDelete.reason); | ||
| this.log('Waiting for other schema registry actions to complete...'); | ||
| result = null; |
There was a problem hiding this comment.
Retrying immediately without any delay creates a tight loop that will hammer the registry API with requests as fast as possible. When implementing a polling mechanism, consider using a longer default interval (e.g., 5-10 seconds) to avoid excessive network requests and potential server load or rate limiting issues.
| if (result.schemaDelete.__typename === 'SchemaDeleteRetry') { | |
| this.log(result.schemaDelete.reason); | |
| this.log('Waiting for other schema registry actions to complete...'); | |
| result = null; | |
| if (result.schemaDelete.__typename === 'SchemaDeleteRetry') { | |
| this.log(result.schemaDelete.reason); | |
| this.log('Waiting for other schema registry actions to complete...'); | |
| await new Promise(resolve => setTimeout(resolve, 5000)); | |
| result = null; |
References
- Focus: Correctness and Performance. Tight loops without delay can cause performance issues and rate-limiting. (link)
- When implementing a polling mechanism, consider the appropriate polling interval to avoid excessive network requests and potential server load or rate limiting issues. A longer default interval (e.g., 5-10 seconds or more) is generally more robust.
There was a problem hiding this comment.
I think it is not needed as the server lock acquiring will only time out after 20 seconds or so.
| """ | ||
| Whether the client supports retries in case the registry is busy and the operation could not be performed. | ||
| """ | ||
| supportsRetry: Boolean = False |
There was a problem hiding this comment.
New clients need to specify that, as old clients do not gracefully handle the SchemaDeleteRetry type member.
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-hive/cli |
0.61.0-alpha-20260717063721-d5dddb2e7d3138bb3c9c545a789354da5bce49f8 |
npm ↗︎ unpkg ↗︎ |
hive |
11.7.0-alpha-20260717063721-d5dddb2e7d3138bb3c9c545a789354da5bce49f8 |
npm ↗︎ unpkg ↗︎ |
21e9c0f to
091a1ad
Compare
091a1ad to
d5dddb2
Compare
|
🐋 This PR was built and pushed to the following Docker images: Targets: Platforms: Image Tags: |
1 participant
Background
For busy projects it is possible that the registry lock acquiring times out. For the schema publishing process we already introduced automatic retries. This also introduces the changes for the schema delete process.
Description
Implement the same behaviour as for schema publishes for retrying from the client side if the registry lock can not be acquired.
Note: I did not add tests as these would require running for a long time (and we also do not have them for the schema publishing counterpart).
I manually tested this be introducing a sleep statement into the schema publish/delete process.