feat(gateway): New federation composition format #4405
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fa4f22f to
3ac7406
Compare
trevor-scheer
added a commit
that referenced
this pull request
Jul 22, 2020
This work is separated out from #4405, and is important to unblock progress on the new composition format. This upgrade could've happened at any time. Now that composition is being updated to a fork of the latest graphql schema printer, v15 is now a requirement. This comes with a few test updates, mostly around the sorting of AST nodes (i.e. in the query plan).
trevor-scheer
added a commit
that referenced
this pull request
Jul 22, 2020
This work is separated out from #4405, and is important to unblock progress on the new composition format. This upgrade could've happened at any time. Now that composition is being updated to a fork of the latest graphql schema printer, v15 is now a requirement. This comes with a few test updates, mostly around the sorting of AST nodes (i.e. in the query plan).
2be570a to
37aca46
Compare
0e1c6d1 to
c085470
Compare
This commit introduces a fresh copy of graphql-js schema printer. Copying it here as the first commit gives us a good diffing point for review.
c085470 to
d998e06
Compare
trevor-scheer
commented
Jul 24, 2020
| @@ -1,87 +1,119 @@ | |||
| /* | |||
There was a problem hiding this comment.
Reviewer: this entire file change is a copy/paste of newPrintFederatedSchema.ts from previous commits. Nothing from this file change is new code - this is the drop-in update for the schema printer.
61a2c39 to
71f5b58
Compare
abernix
reviewed
Jul 24, 2020
There was a problem hiding this comment.
😍 Wow, what an incredible PR! This is super exciting, and very well organized / easy to review. The annotations explaining the federated adjustments are super useful for posterity, too.
(Glad we've brought in updated, upstream printer bits too!)
Two bits:
- I believe that
newPrintFederatedSchema.tsis equal toprintComposedSdl.tsby the end of this. I don't seenewPrintFederatedSchema.tsbeingrequire'd /import'd anywhere; is there meant to be one final commit that removesnewPrintFederatedSchema.ts? - I had one naming thought I mentioned in Directives for SDL representation of composed schema federation#61 (comment) that might be worth tweaking (easy, rename) that I'd be curious in getting your thoughts on.
Copy over the now validated `newPrintFederatedSchema.ts` contents into the former `printFederatedSchema.ts` as an "under-the-hood" swap.
This forks from the newly updated schema printer. However, the goal of this printer is to print the FINAL composition, rather than the schema for a single federated service. By peeking at the existing annotations in the `extensions` on the composed `GraphQLSchema`, the composition printer can gather important metadata like type and field ownership which is then annotated in the final SDL.
71f5b58 to
0784ff7
Compare
abernix
approved these changes
Jul 24, 2020
abernix
pushed a commit
to apollographql/federation
that referenced
this pull request
Sep 4, 2020
…o-server#4405) The output of composition as it stands is not a portable one. In its current form, the output of composition is an executable GraphQLSchema object whose types and fields are annotated via the extensions field. This is problematic for the future of composition at Apollo - as we move query planning to Rust, moving away from this JS native object and towards a portable format (SDL) is ideal, and also allows us to: * parse and consume this format more easily with tooling * create / consume this format in any language which can parse valid GraphQL * standardize the format for other implementors All of that being said, this commit creates an additional output to composition - the printed SDL of the new composition format. We're calling this CSDL C(omposed)SDL to distinguish that this SDL ought to include all of the annotations / metadata necessary about the composed graph in order to perform query planning and execution. Apollo-Orig-Commit-AS: apollographql/apollo-server@1c58ea3
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Design / ref: apollographql/federation#61
Note: this PR is intended to be reviewed by commit. The steps taken to first update the
existing fork and then continue with the new implementation should be fairly clear.
The format of composition as it stands is not a portable one. In its current form,
the result of composition is an executable
GraphQLSchemaobject whose types and fieldsare annotated via the
extensionsfield.This is problematic for the future of composition at Apollo - as we move query planning to
Rust, moving away from this JS native object and towards a portable format (SDL) is ideal,
and also allows us to:
I imagine there's some overlap in my bullets but the points still hold for us.
All of that being said, this PR creates an additional output to composition - the printed
SDL of the new composition format. We're calling this CSDL C(omposed)SDL to distinguish
that this SDL ought to include all of the annotations / metadata necessary about the
composed graph in order to perform query planning and execution.
I included updating our existing fork of
graphql-jsprintSchemaas part of the worksince it fit nicely into my iterative transition while building the new printer. As a
side effect, this PR also resolves an outstanding issue related to printing block descriptions
which include double quote characters. This issue made such a schema unparseable. cc @watson
Fixes #4400
Fixes #4339
Fixes #3864