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

graph/subgraph misusage errors #459

Merged
merged 5 commits into from
May 11, 2021
Merged

graph/subgraph misusage errors #459

merged 5 commits into from
May 11, 2021

Conversation

JakeDawkins
Copy link
Contributor

@JakeDawkins JakeDawkins commented Apr 20, 2021

  • Add error for attempts to introspect a subgraph on graphs which don't support _service (i.e. not spec-compliant subgraphs)
  • Add error for attempts to push a subgraph to a non-federated graph (maybe?)
  • Add error for attempts to run subgraph check on a non-federated graph

Fixes #121

@JakeDawkins JakeDawkins linked an issue Apr 21, 2021 that may be closed by this pull request
@EverlastingBugstopper
Copy link
Contributor

What I'd love to see for rover is the following happy path, assuming mygraph is a federated graph

create a new subgraph: rover subgraph create mygraph --name my-new-subgraph --schema ./my-new-subgraph.graphql --routing-url https://my-new-subgraph.example.com/api/graphql

publish a new schema without updating the routing url: rover subgraph publish mygraph --name my-new-subgraph --schema ./my-new-subgraph.graphql

publish a new schema AND update the routing url rover subgraph publish mygraph --name my-new-subgraph --schema ./my-new-subgraph.graphql --routing-url https://my-new-endpoint.example.com/api/graphql

--

If you tried to rover subgraph publish and the subgraph name did not exist, you'd get an error message pointing you to rover subgraph create

--

If you tried to rover subgraph publish to a graph that exists but is not federated, you'd get an error and be told to create a new federated graph (could maybe have a way to override this if you Really Wanted To convert a graph from non-federated to federated, but imo this feels like a footgun)

@JakeDawkins JakeDawkins changed the title WIP subgraph/subgraph misusage errors WIP graph/subgraph misusage errors Apr 28, 2021
@JakeDawkins JakeDawkins added this to the May 11 - GA milestone Apr 28, 2021
@JakeDawkins
Copy link
Contributor Author

@lrlna I think we agreed we wanted to add a flag to confirm the conversion from monolith to federated graph.

For monolithic graphs or variants with nothing pushed to it, we didn't want to force someone to confirm the change, since it's not destructive of anything. But for existing monoliths, we did want them to confirm with a flag --convert or something maybe?

We said this would take a quick roundtrip to the API to see if the graph was federated to begin with. Probably using the Query.graph.implementingServices, and checking to see if there are any and what kind they are (federated/non-federated).

I hope this makes sense :)

@lrlna lrlna marked this pull request as ready for review May 11, 2021 09:41
@lrlna lrlna changed the title WIP graph/subgraph misusage errors graph/subgraph misusage errors May 11, 2021
@lrlna lrlna merged commit bb81679 into main May 11, 2021
@lrlna lrlna deleted the jake/subgraph-errors branch May 11, 2021 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better errors for using subgraph * commands on non-federated graphs
3 participants