Conversation
ghost
commented
Jan 15, 2020
ghost
commented
|
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/graphprotocol/rfcs/jq1e55ivb |
| <dd><a href="../rfcs/0001-subgraph-composition.md">RFC-0001 Subgraph Composition</a></dd> | ||
|
|
||
| <dt>Engineering Plan pull request</dt> | ||
| <dd><a href="">Engineering Plan PR</a></dd> |
There was a problem hiding this comment.
We can add https://github.com/graphprotocol/rfcs/pull/12 as the href URL now.
|
|
||
| ## Summary | ||
|
|
||
| Since subgraph composition allows a subgraph to import types from another subgraph, GraphQL query execution will need to traverse a subgraph's import graph to resolve concrete types. |
There was a problem hiding this comment.
It's always good to provide some context / what this plan is about before diving straight into details. A brief paragraph saying that this plan covers the query execution part of the Subgraph Composition RFC would be good.
|
|
||
| Since subgraph composition allows a subgraph to import types from another subgraph, GraphQL query execution will need to traverse a subgraph's import graph to resolve concrete types. | ||
| Because imported schemas/types are loosely coupled, query execution also needs to address missing types in the api schema during resolution. | ||
| In addition, imported types which have been renamed need to be mapped back to their original name so that queries can resolve correctly. |
There was a problem hiding this comment.
renamed -> imported with an alias
| 1. Support Custom Type Names: Ensure @originalName directives on the imported types are used to query the correct sql tables. | ||
| 2. Codify New Query Runtime Errors: Ensure @placeholder types are acknowledged and proper errors are returned during query execution when a type's subgraph is not deployed to the graph-node resolving the query. | ||
| 3. Query Correct Subgraph: Ensure that @subgraphId directives on imported types are used correctly in all cases. | ||
| 4. Add Subgraph Composition Feature Flag: Ensure subgraphs JSONB storage can not use subgraph composition. |
There was a problem hiding this comment.
Let's escape directive names with backticks here to render them as inline code.
| 1. Support Custom Type Names: Ensure @originalName directives on the imported types are used to query the correct sql tables. | ||
| 2. Codify New Query Runtime Errors: Ensure @placeholder types are acknowledged and proper errors are returned during query execution when a type's subgraph is not deployed to the graph-node resolving the query. | ||
| 3. Query Correct Subgraph: Ensure that @subgraphId directives on imported types are used correctly in all cases. | ||
| 4. Add Subgraph Composition Feature Flag: Ensure subgraphs JSONB storage can not use subgraph composition. |
There was a problem hiding this comment.
I think this isn't really a feature flag. Since no new subgraphs will be deployed using the JSONB schema and composition isn't yet possible, I think an unreachable!() in the right place may suffice.
| ### Add Subgraph Composition Feature Flag | ||
|
|
||
| Do not allow subgraph composition for subgraphs which use the JSONB storage engine. |
There was a problem hiding this comment.
Like I pointed out, I don't think this is a feature flag?
| ## Tests | ||
|
|
||
| ### Unit Tests: | ||
|
|
|
|
||
| ### Integration Tests: | ||
|
|
||
| 1. Querying an A || [A] with an imported field B || [B] with an imported field C |
There was a problem hiding this comment.
I don't know what this means.
We have support for integration tests via graph test now. I'd say we want a few tests that deploy and query subgraphs that use composition. Tests that involve imported subgraphs being either absent or present. I can help with writing those tests (since this will be new to you), but an overview of the tests that we'd want should be written down here.
| - Support custom type names (1d) | ||
| - Codify new query execution runtime errors (1d) | ||
| - Add subgraph composition feature flag (1d) | ||
| - Ensure correct subgraph is queried during single object resolution and prefetch paths (3d) |
There was a problem hiding this comment.
Missing:
- Integration tests (estimate?)
And I'm not sure the feature flag thing is accurate.
| - Add subgraph composition feature flag (1d) | ||
| - Ensure correct subgraph is queried during single object resolution and prefetch paths (3d) | ||
|
|
||
| ## Open Questions |
There was a problem hiding this comment.
If there are none, either put a paragraph below saying None or drop the section.
1 participant
