Repository navigation
Let a module declare the caches it loads through - #1373
Open
jwrosewell wants to merge 57 commits into
Open
jwrosewell wants to merge 57 commits into
jwrosewell wants to merge 57 commits into
Conversation
A deployment selects a loading cache for each feature in its own [cache.<name>] table, and core listed one feature, robots-txt, so a module's own cache was never selected and every one of its loads ran on every call. A builder now declares the caches its modules load through, by name, [cache.<name>] is read for each declared name with the same settings as [cache.robots-txt], and a table naming a cache no linked module declares is refused when the deployment starts, naming the ones that are declared.
A cache name no linked module declares is refused by validate_settings_for_deploy_with as it is when the caches are made, so a misspelt [cache.<name>] fails ts config validate and not the deployment. A builder declaring robots-txt, or a name another builder declares, is refused naming the builders. The seam probe declares seam-probe-answers and offers a loading cache of its own, and the Axum seam tests prove the table selects it from outside core. The guide names a fictional module.
The debug form of a report carries a backtrace where the test runner asks for one, as CI does, and the deploy check and startup raise the refusal on different stacks, so the comparison held only while both were inlined alike. The message is what the test is about.
…by message As with the probe's test: the debug form of a report carries a backtrace where the test runner asks for one, and the deploy check and the cache build raise the refusal on different stacks.
Series 32 (#1373, for issue #1320) had a pull request and an issue but no design document. This one describes what the code does, adapter by adapter, with the configuration and its validation, the consent and privacy position, the failure handling, the rollout for existing deployments and the tests that exist, and it says plainly which joins have no test.
Review round 1 of the design document found three blocking points. A declared table's lifetimes are range-checked and then applied by nothing, two modules of one publisher keep their copies apart only by a naming convention that nothing checks, and the changelog entry left out the public declared field and the Vec return. The document now states each as the code stands, adds the order of a push, the other callers of loading_caches_from, the name forms core does not check, the paths with no test and four open questions, and drops the service call figure, which has no source. The changelog entry now names the declared field and the Vec return.
Three statements that were only partly true now say what the code does, and the round 1 log sentence no longer claims that every code change waits on a decision. The guide, rustdoc and visibility follow-ups are listed under the open questions with their files and lines.
…s do Each of the 53 designs of the stack that carries both sections puts the revision record before the review log, and this one put it after. The text of both sections is unchanged.
…g offers Since the loading cache change, validate_settings_for_deploy_with calls check_loading_cache_modules, so the deploy check refuses a table whose module names a crate that offers no loading cache, as the state build does. Section 2.7 said the state build alone refused it, and section 3 listed the check as missing. Section 3 now keeps the one limit left, that ts config validate refuses a deployment's own loading cache module unless a tool registers it.
jwrosewell
changed the title
jwrosewell
marked this pull request as ready for review
October 19, 2026 20:23
a_cache_the_probe_declares_is_selected_in_its_own_table counted the 51Degrees module as linked by the stock list, which now links no loading cache module. Its first case links a second loading cache module beside the probe's, a test builder offering core's MemoryLoadingCache, so only a table that is read keeps values, and a second case with the same build and no table keeps nothing. With the probe's cache alone, a declared cache with no table takes it, and with no loading cache module linked it keeps nothing, so those two assertions flip. The comment in core above the step for a declared cache with no table says that a deployment that links one loading cache module holds a module's answers, where it said a stock build did.
The configuration guide said that [cache.example-answers] with module = "51degrees" asks the service once however many requests carry the same evidence. A stock build refuses that module, so the guide now says that this holds in a deployment that links crates/loading-cache/51degrees.
13 of 14 tasks
This branch has not been deployed
This file contains hidden or 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
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.
Summary
Changes
crates/trusted-server-core/srcdocs/superpowerscrates/trusted-server-adapter-axum/testscrates/testing/seam-probedocs/guideCHANGELOG.mdCargo.lockCloses
Closes #1320
Spec
Spec: module-cache-declarations
Test plan
CI on this pull request at 56dd3e8 passed all 20 checks.
cargo test-fastly && cargo test-axumcargo clippy-fastly && cargo clippy-axumcargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute serveChecklist
unwrap()in production code (clippy deniesunwrap_usedin CI)logmacros as AGENTS.md asks (notprintln!)On top of the move of the loading cache module to the released caching crate,
fiftyone-caching4.5.13 (#1372). Issue: #1320.What changed after the 51Degrees PR review
Agreed at the review on 23 October, point 2. The 51Degrees loading cache is pluggable through modules, and the publisher asset feature becomes a module.
module = "51degrees"holds answers only in a deployment that linkscrates/loading-cache/51degrees.Fixes found in review
Kept current
mainat 7aa5c99, the changes made lower in the stack andfiftyone-caching4.5.13.What this adds
A module declares the caches it loads through, by name, on its builder,
IntegrationBuilder::with_cache_features(&["example-answers"]). Core reads a[cache.<name>]table for each declared name as it reads its own[cache.robots-txt], and refuses a table whose name no linked module declares, naming the ones it could be, both ints config validateand when the service starts. A name is the module's own: a builder declaring core'srobots-txt, or a name another builder declares, is refused when the caches are resolved, so two vendors never share one cache and its keys without a word.LoadingCacheskeys its selections by name rather than by a fixed list, andCacheSettings::loading_cachesreturns core's own table first and the declared ones after it, in name order. A declared cache with no table, like a table naming no module (#1365), takes the one loading cache module the build links, so a deployment that links one holds a module's answers without a line in the document, and a build that links none or several keeps nothing. No stock build links one, so a deployment that wants copies kept hands a loading cache module's builder to its adapter.That table is refused unless the build links
crates/loading-cache/51degrees, which no stock build does.The seam probe declares
seam-probe-answersand offers a loading cache of its own, and the Axum adapter's seam tests prove from outside core that the table selects it beside a second loading cache module, that a declared cache with no table takes the one module linked and keeps nothing with none or two linked, and that a misspelt table is refused by the deploy check with the message startup gives.Why
A module that holds its own answers, one service answer that every page of a deployment asks for, asked core for the cache of the name it chose, found no table for it, and got the cache that holds nothing, so every request loaded the answer again. Measured on one deployment at six service calls per page request where one is needed, with a loading cache linked and nothing in the document able to say the module's cache existed, because core read a fixed list of names.
What breaks
Nothing in a deployment's documents.
CacheSettingsno longer rejects an unknown[cache.<name>]table when the document is parsed.ts config validaterefuses it instead, as the service does when it starts, if no linked module declares the name.The commits
robots-txtdeclaration is refused, the seam probe declares and offers a cache, and the guide names a fictional modulepub(crate), since only core calls themThe other commits merge the branch below into this one, and through it Tech Lab main at 7aa5c99.
What is not in this pull request
noneincluded, is refused when the service starts.