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

chore: refactor rover-client in pursuit of structured output #557

Merged
merged 23 commits into from
Jul 26, 2021

Conversation

EverlastingBugstopper
Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper commented May 20, 2021

This PR refactors rover-client into a more principled library that could be consumed by external applications. The main takeaway here is: no more codegen types in rover-client's api! everything has its own type now, there is an input and an output struct with stronger types, shared types are defined in rover-client and most have FromStr implementations that are now used directly in structopt, and we do not re-export any auto-generated types which means we will be able to swap out our graphql clients at will without worrying about breaking changes.

This lays the ground work for structured output, I will have a follow-up PR to add a --json flag that should be able to utilize all of the work that's gone into this refactor and will make consuming that JSON much easier and more consistent than if we had tried this prior to the refactor.


Additionally, I've started printing out the error codes associated with composition errors, just like rover supergraph compose does, which fixes #508.

$ rover supergraph compose --config ../supergraph-demo/supergraph.yaml
error[E029]: Encountered 3 build errors while trying to build a supergraph.

Caused by:
    EXTERNAL_UNUSED: [inventory] Product.id -> is marked as @external but is not used by a @requires, @key, or @provides directive.
    KEY_FIELDS_SELECT_INVALID_TYPE: [products] Product -> A @key selects idddd, but Product.idddd could not be found
    KEY_NOT_SPECIFIED: [inventory] Product -> extends from products but specifies an invalid @key directive. Valid @key directives are specified by the originating type. Available @key directives for this type are:
        @key(fields: "idddd")
        @key(fields: "sku package")
        @key(fields: "sku variation{id}")
    
              The subgraph schemas you provided are incompatible with each other. See https://www.apollographql.com/docs/federation/errors/ for more information on resolving build errors.
$ rover subgraph check rover-supergraph-demo --schema ../supergraph-demo/subgraphs/products.graphql --name products
Checking the proposed schema for subgraph products against rover-supergraph-demo@test
error[E029]: Encountered 3 build errors while trying to build subgraph "products" into supergraph "rover-supergraph-demo@test".

Caused by:
    EXTERNAL_UNUSED: [inventory] Product.id -> is marked as @external but is not used by a @requires, @key, or @provides directive.
    KEY_FIELDS_SELECT_INVALID_TYPE: [products] Product -> A @key selects idddd, but Product.idddd could not be found
    KEY_NOT_SPECIFIED: [inventory] Product -> extends from products but specifies an invalid @key directive. Valid @key directives are specified by the originating type. Available @key directives for this type are:
        @key(fields: "idddd")
        @key(fields: "sku package")
        @key(fields: "sku variation{id}")
    
              The changes in the schema you proposed for subgraph products are incompatible with supergraph rover-supergraph-demo@test. See https://www.apollographql.com/docs/federation/errors/ for more information on resolving build errors.

@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/refactor-subgraph-check branch 2 times, most recently from 7d7332d to 716c278 Compare May 21, 2021 17:25
@EverlastingBugstopper EverlastingBugstopper changed the title wip: refactor subgraph check and add line numbers wip: refactor subgraph check May 21, 2021
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/refactor-subgraph-check branch 3 times, most recently from bb864be to 382b668 Compare May 25, 2021 20:08
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/refactor-subgraph-check branch from 382b668 to 4003978 Compare June 11, 2021 21:28
@EverlastingBugstopper EverlastingBugstopper changed the title wip: refactor subgraph check chore: refactor subgraph check Jun 11, 2021
@EverlastingBugstopper
Copy link
Contributor Author

note to me: might want to try to do an e2e test on that GitContext constructor and make sure the proper env vars are being read

@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/refactor-subgraph-check branch from 086980c to a7d9ce3 Compare June 23, 2021 15:16
@EverlastingBugstopper EverlastingBugstopper changed the title chore: refactor subgraph check (wip) refactor rover-client Jun 23, 2021
@EverlastingBugstopper EverlastingBugstopper changed the title (wip) refactor rover-client [wip] refactor rover-client Jun 23, 2021
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/refactor-subgraph-check branch 4 times, most recently from 68fc240 to 02a8e14 Compare June 28, 2021 20:08
This commit does a lot of heavy lifting for the pending rebase.

1) Creates new input and output types in rover-client for subgraph check
2) Moves GitContext out of rover::utils to rover-client::utils
3) Creates error code E029 for composition errors
4) Styles cloud-composition errors like harmonizer
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/refactor-subgraph-check branch from 478dc88 to 8d0aabf Compare June 29, 2021 18:29
@EverlastingBugstopper EverlastingBugstopper changed the title [wip] refactor rover-client chore: refactor rover-client in pursuit of structured output Jul 15, 2021
@EverlastingBugstopper EverlastingBugstopper added this to the July 20 milestone Jul 15, 2021
@EverlastingBugstopper EverlastingBugstopper force-pushed the avery/refactor-subgraph-check branch from e002f37 to a16f4f7 Compare July 15, 2021 17:54
@abernix abernix removed the request for review from JakeDawkins July 21, 2021 14:03
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.

What a feat! This is coming together nicely. Really really into the structure of rover-client now!

@EverlastingBugstopper EverlastingBugstopper merged commit cdd43c1 into main Jul 26, 2021
@EverlastingBugstopper EverlastingBugstopper deleted the avery/refactor-subgraph-check branch July 26, 2021 20:05
EverlastingBugstopper added a commit that referenced this pull request Aug 5, 2021
EverlastingBugstopper added a commit that referenced this pull request Aug 5, 2021
* Revert "chore: refactor rover-client in pursuit of structured output (#557)"

This reverts commit cdd43c1.

* release: v0.10.0

* Revert "Revert "chore: refactor rover-client in pursuit of structured output (#557)""

This reverts commit 27e2772.
EverlastingBugstopper added a commit that referenced this pull request Aug 5, 2021
…557)"

This reverts structured output so we can cut a patch release for v0.1.10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify styling of composition errors
2 participants