Skip to content

feat: schema delete retries #8230

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.

Sign up for GitHub

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

Open
n1ru4l wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: schema delete retries #8230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/late-bats-study.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@graphql-hive/cli': minor
'hive': minor
---

Support retries for deleting a schema in case the registry is busy and locked.
107 changes: 59 additions & 48 deletions packages/libraries/cli/src/commands/schema/delete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Args, Errors, Flags, ux } from '@oclif/core';
import Command from '../../base-command';
import { graphql, useFragment } from '../../gql';
import { DocumentType, graphql, useFragment } from '../../gql';
import * as GraphQLSchema from '../../gql/graphql';
import { graphqlEndpoint } from '../../helpers/config';
import {
Expand Down Expand Up @@ -37,6 +37,9 @@ const schemaDeleteMutation = graphql(/* GraphQL */ `
...RenderErrors_SchemaErrorConnectionFragment
}
}
... on SchemaDeleteRetry {
reason
}
}
}
`);
Expand Down Expand Up @@ -145,60 +148,68 @@ export default class SchemaDelete extends Command<typeof SchemaDelete> {
target = result.data;
}

const result = await this.registryApi(endpoint, accessToken).request({
operation: schemaDeleteMutation,
variables: {
input: {
serviceName: service,
dryRun: flags.dryRun,
target,
let result: DocumentType<typeof schemaDeleteMutation> | null = null;

do {
result = await this.registryApi(endpoint, accessToken).request({
operation: schemaDeleteMutation,
variables: {
input: {
serviceName: service,
dryRun: flags.dryRun,
target,
supportsRetry: true,
},
},
},
});

if (result.schemaDelete.__typename === 'SchemaDeleteSuccess') {
const { errors, changes } = result.schemaDelete;
});

if (errors) {
const unmaskedErrors = useFragment(RenderErrors_SchemaErrorConnectionFragment, errors);
if (unmaskedErrors.edges.length > 0) {
this.log(renderErrors(errors));
if (result.schemaDelete.__typename === 'SchemaDeleteRetry') {
this.log(result.schemaDelete.reason);
this.log('Waiting for other schema registry actions to complete...');
result = null;
Comment thread
n1ru4l marked this conversation as resolved.
} else if (result.schemaDelete.__typename === 'SchemaDeleteSuccess') {
const { errors, changes } = result.schemaDelete;

if (errors) {
const unmaskedErrors = useFragment(RenderErrors_SchemaErrorConnectionFragment, errors);
if (unmaskedErrors.edges.length > 0) {
this.log(renderErrors(errors));
}
}
}

if (changes) {
const unmaskedChanges = useFragment(RenderChanges_SchemaChanges, changes);
if (unmaskedChanges.edges.length > 0) {
this.log('');
this.log(renderChanges(changes));
if (changes) {
const unmaskedChanges = useFragment(RenderChanges_SchemaChanges, changes);
if (unmaskedChanges.edges.length > 0) {
this.log('');
this.log(renderChanges(changes));
}
}
this.log('');

if (result.schemaDelete.valid) {
this.logSuccess(
flags.dryRun
? `Deleting "${service}" will produce a composable graph. But be sure to review changes to ensure this is safe.`
: `Deleted "${service}" from target`,
);
} else {
this.logWarning(
flags.dryRun
? `Your graph will NOT be composable if "${service}" is deleted.`
: `Deleting "${service}" produced an uncomposable graph.`,
);
}
}
this.log('');

if (result.schemaDelete.valid) {
this.logSuccess(
flags.dryRun
? `Deleting "${service}" will produce a composable graph. But be sure to review changes to ensure this is safe.`
: `Deleted "${service}" from target`,
);
} else {
this.logWarning(
flags.dryRun
? `Your graph will NOT be composable if "${service}" is deleted.`
: `Deleting "${service}" produced an uncomposable graph.`,
);
}

this.exit(0);
return;
}

this.logFailure(`Failed to delete "${service}"`);
const errors = result.schemaDelete.errors;
this.exit(0);
} else {
this.logFailure(`Failed to delete "${service}"`);
const errors = result.schemaDelete.errors;

if (errors) {
throw new APIError(renderErrors(errors));
}
if (errors) {
throw new APIError(renderErrors(errors));
}
}
} while (result === null);

@jdolle jdolle Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add throttling, exponential delays, and/or a maximum retry count?

@n1ru4l n1ru4l Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? We want to acquire the lock as soon as possible so we can perform the action. Can you elaborate your reasoning behind throttling and a maximum retry count?

} catch (error) {
if (error instanceof Errors.CLIError) {
throw error;
Expand Down
13 changes: 12 additions & 1 deletion packages/services/api/src/modules/schema/module.graphql.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,14 @@ export default gql`
| GitHubSchemaCheckSuccess
| GitHubSchemaCheckError
union SchemaDeleteResult = SchemaDeleteSuccess | SchemaDeleteError
union SchemaDeleteResult = SchemaDeleteSuccess | SchemaDeleteError | SchemaDeleteRetry
type SchemaDeleteRetry {
"""
The reason for the retry.
"""
reason: String!

@jdolle jdolle Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to push the retry logic to the client, I think we should provide useful information about the retry. E.g. is there a retry limit?
If we just need a field to exist to be able to return this type, then this is fine

@n1ru4l n1ru4l Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field mostly exists because it is required to have a field. There is currently no retry limit (and I see no reason on why there should be as at some point the lock should be able to be acquired unless someone is doing a schema publish every few seconds, which is another issue imho).

}
type SchemaDeleteSuccess {
valid: Boolean!
Expand Down Expand Up @@ -887,6 +894,10 @@ export default gql`
target: TargetReferenceInput
serviceName: ID!
dryRun: Boolean
"""
Whether the client supports retries in case the registry is busy and the operation could not be performed.
"""
supportsRetry: Boolean = false
}
input GitHubSchemaCheckInput {
Expand Down
Loading
Loading