Eagerly initialize SubgraphOperation::parsed field
#5088
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
As a result, `SubgraphOperation::as_parsed()` no longer requires passing the subgrph schema as a parameter. It may still return an error if `init_parsed` was never called, which would indicate a Router bug.
|
@SimonSapin, please consider creating a changeset entry in |
|
CI performance tests
|
…operation-explicit-init
o0Ignition0o
approved these changes
May 23, 2024
SimonSapin
added a commit
that referenced
this pull request
May 23, 2024
PR #5088 changed the signature of a method. It had a green CI run and git succedded at squashing/rebasing it, but the target `dev` branch gained a new call to that method in the mean time so the resulting merge has a compile error. This would have been prevented if we ran CI on exactly what was about to be pushed to `dev`, instead of only on PR branch. The Rust project calls this [the "not rocket science" rule of software engineering](https://graydon.livejournal.com/186550.html).
SimonSapin
added a commit
that referenced
this pull request
May 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Follow-up to #5075.
As a result,
SubgraphOperation::as_parsed()no longer requires passing the subgraph schema as a parameter. However, because schemas are not available from insideDeserializeimpls, anOptionis still used to createSubgraphOperationwithout a parsed schema at first.as_parsed()may still return an error if the newinit_parsed()method was never called, which would indicate a Router bug:init_parsed()is called immediately after query planning and after getting a Redis cache hit.In the call today we discussed another design with separate query plan types: with and without pared subgraph operations. This would statically prevent failure to initialize, but would require a much larger refactor. For example, the generic dual-storage cache that would need to be modified to have a different cached value type for in-memory cache v.s. external Redis cache.