Add composition options to be able to ignore parts of the composition pipeline #3232
Conversation
|
✅ Docs preview has no changesThe preview was not built because there were no changes. Build ID: 1787e33aa6a2f47793ff66c2 |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
| sdlPrintOptions?: PrintOptions; | ||
| allowedFieldTypeMergingSubtypingRules?: SubtypingRule[]; | ||
| /// Flag to toggle if satisfiability should be performed during composition | ||
| runSatisfiability?: boolean; |
There was a problem hiding this comment.
this would be a breaking change.... but ideally this should move under new pipelineOptions?
There was a problem hiding this comment.
It would be a breaking change to add an extra optional field to the return type?
There was a problem hiding this comment.
if was thinking about moving this over but yeah if we also add extra option there then yeah that should be fine (and IMHO more consistent, maybe we could deprecate the old field as well)
There was a problem hiding this comment.
Sorry, which old field do you mean?
There was a problem hiding this comment.
move runSatisfiability?: boolean; under new pipelineOptions
There was a problem hiding this comment.
see my other comment though as I think this PR might not be that useful from hybrid composition perspective
There was a problem hiding this comment.
I thought this PR would make it possible to support the future HybridComposition. Can federation-rs call composition with the piplineOptions to achieve the goals, no? 🤔
There was a problem hiding this comment.
Kind of -> using pipeline options would enable us to skip first or last stages but will not allow pick and choose style of integrations.
i.e. given following stages
pipelineOptions?: {
runUpgradeSubgraphs?: boolean;
runValidateSubgraphs?: boolean;
runComposition?: boolean;
runSatisfiability?: boolean;
outputUpgradedSubgraphs?: boolean;
}How would you integrate it with hybrid composition if you only want to replace subgraph validations with Rust version? Or if you just want to compare single phases?
There was a problem hiding this comment.
I could move it, but I was thinking that runSatisfiability will be more long lived, but the other ones are really only for our use and will be temporary.
|
Was thinking about this more and I am unsure whether this will be that useful, i.e. in pub trait HybridComposition {
/// Call the JavaScript `composeServices` function from `@apollo/composition` plus whatever
/// extra logic you need. Make sure to disable satisfiability, like `composeServices(definitions, {runSatisfiability: false})`
async fn compose_services_without_satisfiability(
&mut self,
subgraph_definitions: Vec<SubgraphDefinition>,
) -> Option<SupergraphSdl>;
/// Call the JavaScript `validateSatisfiability` function from `@apollo/composition` plus whatever
/// extra logic you need.
async fn validate_satisfiability(&mut self) -> Result<SatisfiabilityResult, Issue>;
/// Allows the Rust composition code to modify the stored supergraph SDL
/// (for example, to expand connectors).
fn update_supergraph_sdl(&mut self, supergraph_sdl: String);
fn add_issues<Source: Iterator<Item = Issue>>(&mut self, issues: Source);
/// Runs the complete composition process, hooking into both the Rust and JavaScript implementations.
///
/// # Algorithm
///
/// 1. Run Rust-based connector validation on the subgraphs
/// 2. Call [`compose_services_without_satisfiability`] to run JavaScript-based composition
/// 3. Run Rust-based connector validation on the supergraph
/// 4. Call [`validate_satisfiability`] to run JavaScript-based validation on the supergraph
async fn compose(&mut self, subgraph_definitions: Vec<SubgraphDefinition>) {
// default impl goes here
}
}I think in order to make it usable as pick&choose from the trait we would need to update composition to expose those individual stages so we can easily call those functions directly, i.e. pub trait HybridComposition {
fn process_subgraphs(&mut self, subgraph_definitions: Vec<Subgraph<Raw>>) -> Vec<Subgraph<Expanded>>;
fn validate_subgraphs(&mut self, subgraph_definitions: Vec<Subgraph<Expanded>>) -> Vec<Subgraph<Validated>>;
fn merge(&mut self, Vec<Subgraph<Validated>>) -> Supergraph<Raw>;
fn validate_satisfiability(&mut self, Supergraph<Raw>) -> Supergraph<Validated>;
fn compose(&mut self, subgraph_definitions: Vec<SubgraphDefinition>) {
// default impl goes here -> I think this should return Supergraph<Validated>
}
} |
No description provided.