Conversation
✅ Docs Preview ReadyNo new or changed pages found. |
This comment has been minimized.
This comment has been minimized.
|
CI performance tests
|
b2d86a2 to
9d22321
Compare
|
This is ready for review but should only be merged after we branch 1.59 later this week. |
lrlna
left a comment
There was a problem hiding this comment.
Potentially the most exciting PR in router's history! 🎉
Leaving some initial comments.
| Error: Cannot find type "Review" in subgraph "products" | ||
| caused by | ||
| "# | ||
| Details: Object field "Product.reviews"'s inner type "Review" does not refer to an existing output type."# |
There was a problem hiding this comment.
🤔 is this an issue that the error message is very different?
| let config = format!("{PROMETHEUS_METRICS_CONFIG}\n{NEW_QP}"); | ||
| let mut router = IntegrationTest::builder() | ||
| .config(config) | ||
| .config(PROMETHEUS_METRICS_CONFIG) |
There was a problem hiding this comment.
should we just drop this test? i think this file can just have the fed1 tests and that's it.
There was a problem hiding this comment.
The apollo.router.lifecycle.query_planner.init metric still exists and is tested here. Should we also remove it?
There was a problem hiding this comment.
Hmmm, no we should track that metric for the time being - specifically for fed 1 supergraph fall backs that get added to ...query_planner.init.
% Conflicts: % apollo-router/src/query_planner/bridge_query_planner.rs
garypen
left a comment
There was a problem hiding this comment.
This looks great! We should follow up at some point with some renaming of BridgeQueryPlanner bits and I think we could completely delete BridgeQueryPlannerPool.
* cmake is no longer required to build Router since #6418 * nodejs hasen’t been required for much longer: router-bridge shipped with a pre-built V8 "bundle" * xtask is a pattern not a singular tool: <https://crates.io/crates/cargo-xtask> is just a placeholder so `cargo install cargo-xtask` does nothing

The legacy query planner has been removed in this release. In the previous release, Router 1.58, it was already no longer used by default but it was still available through the
experimental_query_planner_modeconfiguration key. That key is now removed.Also removed are configuration keys which were only relevant to the legacy planner:
supergraph.query_planning.experimental_parallelism: the new planner can always use available parallelism.supergraph.experimental_reuse_query_fragments: this experimental algorithm that attempted toreuse fragments from the original operation while forming subgraph requests is no longer present. Instead, by default new fragment definitions are generated based on the shape of the subgraph operation.
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩