Skip to content
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

fix: make registry-upgrade more tolerant of problems #1715

Merged
merged 1 commit into from
May 14, 2024

Conversation

obmarg
Copy link
Member

@obmarg obmarg commented May 14, 2024

registry-upgrade currently panics if you pass it a registry that references a type that dopesn't exist. The thinking here was that ideally we validate schemas before we even parse them, so that we can give the users a good error message. And then hopefully our code doesn't do anything to make the registry invalid between validation and passing it to registry-upgrade.

This was wishful thinking - I'm not sure we do validate the schema, and parser-sdl (and probably the connectors) are also perfectly capable of generating bad registries.

This updates registry-upgrade to be more tolerant of these kinds of problems:

  1. Where possible it'll just skip any fields with missing types. We could raise these as errors to the user, but I think they're usually a sign of a bug elsewhere - either we're generating invalid registries, or we're not catching invalid schemas. So I kind of think a best effort approach to getting something deployed is appropriate.
  2. If the user doesn't provide a root Query type there's no graceful way to handle that - so for that case I've updated the crate to return an error.

registry-upgrade currently panics if you pass it a registry that references a
type that dopesn't exist.  The thinking here was that _ideally_ we validate
schemas before we even parse them, so that we can give the users a good error
message.  And then hopefully our code doesn't do anything to make the registry
invalid between validation and passing it to registry-upgrade.

This was wishful thinking - I'm not sure we _do_ validate the schema, and
parser-sdl (and probably the connectors) are also perfectly capable of
generating bad registries.

This updates registry-upgrade to be more tolerant of these kinds of problems:

1. Where possible it'll just skip any fields with missing types.  We could
   raise these as errors to the user, but I think they're _usually_ a sign
   of a bug elsewhere - either we're generating invalid registries, or we're
   not catching invalid schemas.  So I kind of think a best effort approach
   to getting _something_ deployed is appropriate.
2. If the user doesn't provide a root `Query` type there's no graceful
   way to handle that - so for that case I've updated the crate to return
   an error.
@obmarg obmarg requested a review from a team as a code owner May 14, 2024 09:11
@obmarg obmarg requested a review from hfgbarrigas May 14, 2024 09:11
Copy link
Contributor

@tomhoule tomhoule left a comment

Choose a reason for hiding this comment

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

Looks good!

@obmarg obmarg merged commit 9639657 into main May 14, 2024
21 checks passed
@obmarg obmarg deleted the obmarg-push-trlpozrsszrm branch May 14, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants