-
Notifications
You must be signed in to change notification settings - Fork 50
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
subschema composability rules #37
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yaacovCR This is great. 👏 I've also been thinking about mergability and a common merging spec as a primary concern.
I've added some of my riffs as suggestions to your riffs 🙂
ea371d8
to
9995117
Compare
to drop the number of used terms/simply, we can just use type composition
1. separate composability rules by execution strategy. I am not sure if these should be separate in the final document, but separating helps me reason about them better within this draft stage. 2. remove discussion of runtime translation from composability rules thats basically mixing the idea of "transforms" with composability. For example, if there is a way of translating a missing enum value for a given subschema into an enum value that is present, we are basically transforming that subschema into a subschema that has the missing enum value by using a transformRequest method Instead, let's make the composability rules strict and discuss separately the idea of transforms
I have marked this PR as no longer DRAFT, as I have added section regarding all types. But it would certainly benefit from additional input and further revision. Comments are welcome. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had some more thoughts on "what does it mean to be composable?". This feels like it could be a whole separate topic, so I'm happy to move this to a discussion too.
This is a great doc! I don't find the GitHub PR interface great for reviewing Markdown, so I wrote up a few thoughts in the discussion thread: |
Minor markdown styling comment: please use
and
i.e. change
to
This follows the pattern used by spec-md. |
Thanks @michaelstaib Co-authored-by: Michael Staib <[email protected]>
Thanks @michaelstaib Co-authored-by: Michael Staib <[email protected]>
@benjie any styling for definitions like for “composability” ? |
@yaacovCR Indeed; if you're actually using spec-md then the paragraph that defines a term should start with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we go ahead and merge this since it's an RFC doc?
subschemas. This set of subschemas is then considered to be `composable`. | ||
Inversely, it may be impossible to generate a valid composite schema containing | ||
all of the GraphQL elements of a given set of subschemas; this set of subschemas | ||
is considered `not composable`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mathematically speaking, isn't it always possible to generate a composite schema from any two schemas A and B that implement the same version of the GraphQL spec and whose root types don't implement any interfaces by renaming every non-standard non-root type and directive in them, prefixing the names of all the fields on the root types with the name of the schema when adding them to the composite schema's root types, and replacing references to the root types in the sub-schemas with the respective composite schema's root types? Sure this becomes A + B rather than A & B - i.e. each field is resolved by exactly one subschema - but that's not really at odds with this paragraph?
I think the wording here needs to be more crisp... not sure how exactly - perhaps refer to the overlapping elements somehow?
composite schema will yield request(s) against spec-compliant (or even | ||
non-spec compliant) GraphQL subservices that are then merged into a single | ||
request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term "GraphQL" implies it's spec compliant. If they're not spec compliant then they're not GraphQL.
Also do you mean "result" rather than "request" here?
composite schema will yield request(s) against spec-compliant (or even | |
non-spec compliant) GraphQL subservices that are then merged into a single | |
request. | |
composite schema will yield request(s) against GraphQL subservices that | |
are then merged into a single result. |
"build-time" composition, a single service executes the composite schema | ||
without delegation to remote services tied to each subschema. Subschemas | ||
exist for the purpose of code organization, but only the composite schema | ||
exists at the time of execution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exists at the time of execution. | |
exists at the time of execution. | |
3. `Hybrid execution`: a combination of the above approaches. |
The below RFC examines composability separately for the above two forms of | ||
composite schema execution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you accept my Hybrid
suggestion above, this will need adjusting.
example, if a request against the composite schema must ultimately be translated | ||
to request(s) against the still extant source subschemas, then the composition | ||
must be -- in some sense -- reversed during execution, which is not always | ||
possible, and introduces significant complexity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Under what circumstances would it be something that we'd do and yet it wouldnot be possible to reverse?
example, if a request against the composite schema must ultimately be translated | |
to request(s) against the still extant source subschemas, then the composition | |
must be -- in some sense -- reversed during execution, which is not always | |
possible, and introduces significant complexity. | |
example, if a request against the composite schema must ultimately be translated | |
to request(s) against the still extant source subschemas, then the composition | |
must be -- in some sense -- reversed during execution, which introduces | |
complexity. |
to different types in the composite schema. Or, when metadata denotes types as | ||
overlapping in the first instance, namespacing may be accomplished simply by | ||
adjusting the existing metadata or via additional metadata. Finally, a new | ||
namespace construct may be introduced into the GraphQL specification itself such |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
namespace construct may be introduced into the GraphQL specification itself such | |
namespace construct might be introduced into the GraphQL specification itself such |
1. Overlapping enum types where the types define identical sets of values can | ||
always be composed, as they are identical in all subschemas. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A risk of this, however, is if you compose them as equivalent but it turns out they have different meanings and they later wish to diverge. For example:
enum Can {
DUCK
}
might evolve in one schema (capabilities) to enum Can { DUCK RUN }
🏃 or in another (canned goods) to enum Can { DUCK BEANS }
🥫
2. Subschemas with overlapping enum types where the disparate types define | ||
different value sets are only sometimes composable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. Subschemas with overlapping enum types where the disparate types define | |
different value sets are only sometimes composable. | |
2. Subschemas with overlapping enum types where the disparate types define | |
different value sets are sometimes composable. |
1. Overlapping interface types cannot be composed if any overlapping fields | ||
cannot be composed, see below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May need a comment on the interfaces that the interface implements.
1. Overlapping object types cannot be composed if any overlapping fields cannot | ||
be composed, see below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a comment on the interfaces the object type implements too.
5aab83d
to
a48b466
Compare
During our call today we outlined where we're going, and sounds like composability rules are on the horizon again. @michaelstaib @martijnwalraven We could use Yaacov's PR here as a discussion starter. |
in the rfc there were several times mentioned ways to influence schema objects composability
As far as I understood, it must be some directives, which allow merging or do merging implicitly? May small paragraph on directives which supply merging will be useful there? (please just ignore my input, if it doesn't help!) |
Comments welcome!
This is just an initial version, and has much room for improvement.