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

feat(spike): rover core build #340

Merged
merged 20 commits into from
Mar 22, 2021
Merged

feat(spike): rover core build #340

merged 20 commits into from
Mar 22, 2021

Conversation

EverlastingBugstopper
Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper commented Mar 12, 2021

This PR introduces a YAML config format for specifying multiple subgraphs to compose.

example YAML with two subgraphs titled films and people:

subgraphs:
  films:
    routing_url: https://films.example.com
    schema: 
      file: ./good-films.graphql
  people:
    routing_url: https://people.example.com
    schema: 
      file: ./good-people.graphql
$ rover core build --help
rover-core-build 0.0.3
Build a core schema from a set of subgraphs

USAGE:
    rover core build [OPTIONS] --config <core-config>

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

OPTIONS:
        --config <config-path>    The relative path to the core configuration file
    -l, --log <log-level>          [possible values: error, warn, info, debug, trace]

This will attempt to compose your subgraph schemas. If it successfully composes, it will print the composed CSDL. Paths are computed relative to the config file itself. We could eventually look for a rover.toml by default, but for now, since this is the only config file we have and nothing has been set in stone for other commands, we require an argument that takes a relative path.

Example:

$ rover core build --config spike/composes.yaml
CSDL:
schema
 @graph(name: "films", url: "https://films.example.com")
 @graph(name: "people", url: "https://people.example.com")
 @composedGraph(version: 1)
{
 query: Query
}

directive @composedGraph(version: Int!) on SCHEMA

directive @graph(name: String!, url: String!) repeatable on SCHEMA

directive @owner(graph: String!) on OBJECT

directive @key(fields: String!, graph: String!) repeatable on OBJECT

directive @resolve(graph: String!) on FIELD_DEFINITION

directive @provides(fields: String!) on FIELD_DEFINITION

directive @requires(fields: String!) on FIELD_DEFINITION

type Film {
 id: ID!
 title: String
 actors: [Person]
 director: Person
}

type Person
 @owner(graph: "people")
 @key(fields: "{ id }", graph: "people")
 @key(fields: "{ id }", graph: "films")
{
 id: ID!
 name: String
 appearedIn: [Film] @resolve(graph: "films")
 directed: [Film] @resolve(graph: "films")
}

type Query {
 film(id: ID!): Film! @resolve(graph: "films")
 films: [Film] @resolve(graph: "films")
 person(id: ID!): Person @resolve(graph: "people")
 people: [Person] @resolve(graph: "people")
}

and

$ cargo run -- core build --config spike/errors.yaml
error: KEY_FIELDS_MISSING_EXTERNAL: [films] Person -> A @key directive specifies a field which is not found in this service. Add a field to this type with @external.
error: KEY_FIELDS_SELECT_INVALID_TYPE: [films] Person -> A @key selects idd, but Person.idd could not be found
error: KEY_NOT_SPECIFIED: [films] Person -> extends from people but specifies an invalid @key directive. Valid @key directives are specified by the originating type. Available @key directives for this type are:
        @key(fields: "id")
error: Encountered 3 composition errors while composing the graph.

Error cases we need to handle better we currently handle:

  • Config file cannot be found
  • Config file cannot be read to a string
  • Config file cannot be parsed
  • Schema path defined in the config file cannot be found
  • Schema file cannot be read to a string

^ These are all handled now!

  • Probably others I'm not thinking of

^ I'd love some QA on this to see if folks can break it and we can add more test cases 😄


It's important to note that we do absolutely no input validation on the URLs due to the fact that I think they can really be anything, even empty strings. That being said, we do require that it is defined for each subgraph.

There are three options here:

  • Change nothing in this PR, allow any string to be passed, but do not parse it or require it to be well-formed.
  • Do not require a routing_url for each subgraph and pass an empty string to harmonizer if it is not specified in the config file.
  • Be more strict/opinionated about routing_url and throw a parse error if a routing_url does not exist or if it's not a well-formed URL that would produce a bad artifact.

Closes #350

@EverlastingBugstopper EverlastingBugstopper added the feature 🎉 new commands, flags, functionality, and improved error messages label Mar 12, 2021
@EverlastingBugstopper EverlastingBugstopper self-assigned this Mar 12, 2021
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/sure-why-not branch 2 times, most recently from 71684b2 to 196f113 Compare March 16, 2021 20:42
@EverlastingBugstopper EverlastingBugstopper changed the title feat(spike): rover graph build feat(spike): rover core build Mar 17, 2021
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/sure-why-not branch 2 times, most recently from e0ea5c6 to 5112544 Compare March 17, 2021 15:34
@EverlastingBugstopper EverlastingBugstopper marked this pull request as ready for review March 17, 2021 15:34
spike/composes.yaml Outdated Show resolved Hide resolved
this PR introduces a TOML config format for specifying multiple
subgraphs to compose.

usage: rover graph build --services ./path/to/services.toml

This will attempt to compose your subgraph schemas. If it successfully composes,
it will print the composed CSDL.

If there are errors, they will be printed.

examples of both composing and non-composing inputs can be tested by running:

`cargo run -- graph build --services spike/composes.toml`

and

`cargo run -- graph build --services spike/errors.toml`
@abernix abernix mentioned this pull request Mar 18, 2021
@EverlastingBugstopper
Copy link
Contributor Author

EverlastingBugstopper commented Mar 18, 2021

from @ndintenfass in Slack:

This looks great - I propose we change schema_path a bit, either to shorten it in place, or maybe something that has more flexibility like:

subgraphs:
  films:
    routing_url: https://films.example.com
    schema:
      file: ./good-films.graphql
  people:
    routing_url: https://people.example.com
    schema: 
      file: ./good-people.graphql 

That would let file become something like graph or introspection_url (or even raw per earlier discussion that if all of them resolved to the raw internally people could build their own tooling in a pinch)

for instance, then I could later do something like:

subgraphs:
  films:
    routing_url: https://films.example.com
    schema:
      file: ./good-films.graphql
  people:
    routing_url: https://people.example.com
    schema: 
      graph: mygraph@current
      subgraph: good-people

Copy link
Member

@lrlna lrlna left a comment

Choose a reason for hiding this comment

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

Let's bring this in as part of 0.0.4, so folks can try it out before our release next week.

@JakeDawkins JakeDawkins merged commit 5697962 into main Mar 22, 2021
@JakeDawkins JakeDawkins deleted the avery/sure-why-not branch March 22, 2021 18:38
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 this pull request may close these issues.

Composition
3 participants