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

Composition from running services and/or registered graphs or subgraphs #449

Closed
lrlna opened this issue Apr 16, 2021 · 12 comments · Fixed by #519
Closed

Composition from running services and/or registered graphs or subgraphs #449

lrlna opened this issue Apr 16, 2021 · 12 comments · Fixed by #519
Labels
feature 🎉 new commands, flags, functionality, and improved error messages
Milestone

Comments

@lrlna
Copy link
Member

lrlna commented Apr 16, 2021

Description

Extend the YAML config for composition to include both URLs of running subgraphs (the "service SDL introspection") or registered subgraphs.

Proposed implementation

this is written as if it's a replacement in docs for https://www.apollographql.com/docs/rover/supergraphs/#configuration

When composing a supergraph schema you will specify a set of subgraphs to compose in a YAML file.

These subgraphs can be pulled from any of the following sources:

  • Local files
  • Running GraphQL servers that implement the federation spec (eg: Apollo Server and others)
  • Registered schema in the Apollo registry, using a graph ref

An example below shows three subgraphs, each pulled from a different place. Look at the different values under the schema to understand the options:

subgraphs:
  films:
    routing_url: https://films.example.com
    schema:
      file: ./films.graphql
  people:
    schema: 
      url: https://people.example.com
  reviews:
    schema:
      graphref: mygraph@current
      subgraph: reviews    

Details of each schema stanza option follow:

Pulling schema from a local file

schema:
  file: ./films.graphql

Note that when using a file the key routing_url is required as a peer to schema to specify where queries will be routed in your gateway/router.

(Known error states: the file cannot be found, the file does not contain a valid GraphQL schema document)

Pulling schema from a URL via introspection

schema: 
  url: https://url.to.running.subgraph

TODO: Based on the way Workbench does this, the above shape implies you use federation subgraphs OR regular GraphQL introspection interchangeably, with order of precdence - we need to decide if we should distinguish semantics of a subgraph introspection vs. a regular introspection, or do this magically like Workbench. This was controversial when talking with our internal Solutions team -- there's value in the magic, but it also creates a risk of non-determinant schema for the same input

Specifying a URL as your schema source will attempt to use introspection to get the schema. If the underlying server supports the federation specification (eg: Apollo Server or others) the schema will contain the federation directives. If it does not, regular GraphQL introspection will be used.

Note that when using url as your source the routing_url is optional and will be assumed to be the same value as url.

(Known error states: the URL is malformed, the URL cannot be reached [no HTTP response], the URL is not answering introspection, the URL is 404 [or any non-200 http code?])

Pulling subgraph schema from the Apollo registry

schema:
  graphref: mygraph@current
  subgraph: studios    

TODO: Here we are assuming we can use the subgraph's registered routing_url, which is great in the base case, but we also do want to allow you to pull a regular API schema at a graphref without specifying a subgraph, and it would be potentially nice to have that be a graphref with no subgraph, but then we DO need to require the routing_url in that case, which is a bit convoluted

In the above example mygraph@current is a graph ref to a registered schema in Apollo. The subgraph is the name of the subgraph you want to reference.

Note that when using url as your source the routing_url is optional and will be assumed to be the URL associated with the subgraph in Studio.

(Known error states: the graphref is invalid, the subgraph does not exist on the graphref)

@lrlna lrlna added feature 🎉 new commands, flags, functionality, and improved error messages triage issues and PRs that need to be triaged labels Apr 16, 2021
@lrlna lrlna added this to the May 11 - GA milestone Apr 16, 2021
@lrlna
Copy link
Member Author

lrlna commented Apr 16, 2021

@ndintenfass I copied this over from the roadmap draft. If you can, please add specific requirements and goals you'd like us to achieve with this feature.

@ndintenfass
Copy link
Contributor

@lrlna I added some details

@lrlna lrlna removed the triage issues and PRs that need to be triaged label Apr 16, 2021
@lennyburdette
Copy link
Contributor

After literally sleeping on it, here's a suggestion:

subgraphs:
  films:
    routing_url: https://films.example.com
    schema:
      graphref: mygraph@current
      subgraph: films

versus

subgraphs:
  films:
    routing_url: https://films.example.com
    schema:
      graphref: mygraph@current
      as_subgraph: true

The former case (including a subgraph from studio in composition) is the primary use-case right? The latter case is useful for migrating from monoliths or experimenting, but will be uncommon. Making the less-common use-case easier to configure than the more common use-case might lead to confusion.

I can imagine an error message like this if you forget to add the subgraph: or as_subgraph: true:

ERROR: Please specify a subgraph of `mygraph@current` to compose. Did you mean to include `subgraph: films`?

If you intend to include the entire `mygraph@current` as a subgraph, add `as_subgraph: true`.

@EverlastingBugstopper
Copy link
Contributor

@lennyburdette this is interesting! I hadn't thought of this use case, though it seems like it would add some interesting (but.. possibly confusing?) possibilities.

Just to clarify to make sure I'm on the same page about this - the second option allows you to treat a graph that's tracked in the registry as a subgraph input for local composition?

If so, an edge case to think about there is if someone specifies a fully federated graph as a subgraph with as_subgraph: true. I.. would assume it would work OK as long as we fetched SDL and not the Core Schema (though we could maybe get into some super weird circular dependency type situations).

@lennyburdette
Copy link
Contributor

Yeah, @ndintenfass suggested that option. I don't quite understand how it would work under the hood, but seems like something that should be possible as composition gets more sophisticated/flexible.

@jsegaran
Copy link
Contributor

If we encode subgraphs into schema refs like mygraph@current^films, I don't think we will need to have the subgraph field?
This would work nicely with graphs composing graphs where all subgraphs would be their own graph. If we allow users to provide an arbitrary ref to fetch a schema with we can support people more use cases I think?

@ndintenfass
Copy link
Contributor

ERROR: Please specify a subgraph of `mygraph@current` to compose. Did you mean to include `subgraph: films`?

If you intend to include the entire `mygraph@current` as a subgraph, add `as_subgraph: true`.

@lennyburdette this makes sense - my general principle is to minimize both the number of semantically meaningful tokens in a DSL and the number of combinatorial states of subkeys. Arguably, having 0 or 1 values is the same level of complexity as 1 or two specific values, but in this case since the top-level key is already subgraphs we're already implicitly in the scope of specifying subgraphs, so as_subgraph feels like redundant information only to force "another" value, but the absence of a value seems just as semantically meaningful without needing to understand the nature of the conceptual translations that "as" implies in `as_subgraph.

@ndintenfass
Copy link
Contributor

ndintenfass commented Apr 28, 2021

@jsegaran we've been explicitly punting on trying to get subgraphs into the graph ref for a couple reasons:

  1. It makes the graph ref another degree of semantically complex -- means you have to understand a 3-level semantics that is encoded by single characters -- has a bit of a perl-like quality in that way. You can also credibly say that mygraph@current^mysub is semantically equivalent to mygraph^mysub, which means to "read" the code where refs appear I need to really understand the domain model and how its encoded in the ref. And since the API doesn't talk that way anyway, it also means that much more parsing in the client.
  2. When we do graphs composing the graphs the subgraph will only be interesting when reading it back out for diagnostic reasons, so needing to always document the graph ref to include them means we have to carry that around long after it's less useful.

@ndintenfass
Copy link
Contributor

One bit of feedback I got when shopping this around was to have a nice way to "watch" for changes to schema and then recompose automatically -- I added that as a new ticket, as it seems out of scope for this ticket, but is related: #479

@ndintenfass
Copy link
Contributor

Another bit of feedback I got was that it would be nice for rover to help you bootstrap a config file, including from an existing federated graph in the registry. That's out of scope here, so I made a new issue: #480

@ndintenfass
Copy link
Contributor

(Also note that I substantially updated the description of this issue)

@lrlna
Copy link
Member Author

lrlna commented May 7, 2021

There is a Draft version of this implementation over at #519, if any of you are keen to try this out early.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🎉 new commands, flags, functionality, and improved error messages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants