BE-802: Load programmatic defaults in hash-config - #9437
Conversation
`Loader::with_defaults` takes anything that serializes to a map and merges it into the default layer; repeated calls replace scalars and arrays and merge maps recursively. `Loader::load` deserializes the merged values. Errors report the key and the expected shape and never the value, so a report is safe to log in full: a mistyped `store.port` reads `invalid type: found string, expected u16 at "store.port"`. Configuration keys count as schema and are reported, including map keys and array indices; scalars, enum variant names, lengths and hand-written Serde messages are values and are dropped. Figment's `Debug` renders the merged values, so `Loader` implements it by listing the layer names instead.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
Signed-off-by: Tim Diekmann <21277928+TimDiekmann@users.noreply.github.com>
Merging this PR will degrade performance by 15.38%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | bit_matrix/dense/iter_row[64] |
140.8 ns | 170 ns | -17.16% |
| ❌ | bit_matrix/dense/iter_row[200] |
185.8 ns | 215 ns | -13.57% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing t/be-802-load-programmatic-defaults-in-hash-config (0968bae) with main (59468b1)1
Footnotes
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9437 +/- ##
==========================================
+ Coverage 60.66% 61.01% +0.35%
==========================================
Files 1439 1448 +9
Lines 143119 144648 +1529
Branches 6654 6692 +38
==========================================
+ Hits 86817 88263 +1446
- Misses 55211 55281 +70
- Partials 1091 1104 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Four doc comments described a mechanism or justified a choice instead of saying what the item is: on `Defaults::new`, on `Debug for Loader`, on a test, and in the provider's `metadata`. What `#[track_caller]` does needs no comment. `Loader` is documented as building from layered sources rather than from explicitly selected ones, and `with_defaults` says the error names the call instead of saying that a call site is recorded.
A key that came from a selected profile reports it, so `missing field `port`` becomes `missing field `port` under profile `production``. The default profile is left unnamed: until a profile is selected every key sits under it.
PR SummaryLow Risk Overview Failures return Adds Reviewed by Cursor Bugbot for commit 0968bae. Bugbot is set up for automated code reviews on this repo. Configure here. |
Benchmark results
|
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2002 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 1002 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 3314 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 1527 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 2078 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 1033 | Flame Graph |
policy_resolution_medium
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 102 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 269 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 108 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 133 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 63 | Flame Graph |
policy_resolution_none
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 8 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 3 | Flame Graph |
policy_resolution_small
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 26 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 94 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 27 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 66 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 29 | Flame Graph |
read_scaling_complete
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id;one_depth | 1 entities | Flame Graph | |
| entity_by_id;one_depth | 10 entities | Flame Graph | |
| entity_by_id;one_depth | 25 entities | Flame Graph | |
| entity_by_id;one_depth | 5 entities | Flame Graph | |
| entity_by_id;one_depth | 50 entities | Flame Graph | |
| entity_by_id;two_depth | 1 entities | Flame Graph | |
| entity_by_id;two_depth | 10 entities | Flame Graph | |
| entity_by_id;two_depth | 25 entities | Flame Graph | |
| entity_by_id;two_depth | 5 entities | Flame Graph | |
| entity_by_id;two_depth | 50 entities | Flame Graph | |
| entity_by_id;zero_depth | 1 entities | Flame Graph | |
| entity_by_id;zero_depth | 10 entities | Flame Graph | |
| entity_by_id;zero_depth | 25 entities | Flame Graph | |
| entity_by_id;zero_depth | 5 entities | Flame Graph | |
| entity_by_id;zero_depth | 50 entities | Flame Graph |
read_scaling_linkless
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 100 entities | Flame Graph | |
| entity_by_id | 1000 entities | Flame Graph | |
| entity_by_id | 10000 entities | Flame Graph |
representative_read_entity
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1
|
Flame Graph |
representative_read_entity_type
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| get_entity_type_by_id | Account ID: bf5a9ef5-dc3b-43cf-a291-6210c0321eba
|
Flame Graph |
representative_read_multiple_entities
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_property | traversal_paths=0 | 0 | |
| entity_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=0 | 0 | |
| link_by_source_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true |
scenarios
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| full_test | query-limited | Flame Graph | |
| full_test | query-unlimited | Flame Graph | |
| linked_queries | query-limited | Flame Graph | |
| linked_queries | query-unlimited | Flame Graph |
🌟 What is the purpose of this PR?
hash-configgets its first working slice. A binary builds its configuration type from programmatic defaults withLoader::new().with_defaults(…).load::<Config>(), which returns the deserialized configuration. Figment does the merging and appears in no public signature.The error path is the substantive part of this change. Figment's own messages inline the value they rejected, so a bad
store.portreadsinvalid type: found string "hunter2", expected u16. A configuration value can be a database password or an API key, which makes such an error unsafe to put in a log. Here the error names the key and the shape that was expected, and never the value.🔗 Related links
🚫 Blocked by
Nothing. BE-801 is merged.
🔍 What does this change?
Loader::with_defaultsaccepts anyimpl Serializethat serializes to a map and merges it into the default layer. Repeated calls apply in order: later scalars and arrays replace earlier ones, and maps merge recursively. BE-802 calls this methoddefaults; it iswith_defaultshere to match the builder convention the rest of the workspace uses, including the layered builder inlibs/@local/telemetry/src/lib.rs.Loader::loaddeserializes the merged values into the caller's type and returnsReport<LoadError>on failure.missing field \store.port`rather thanmissing field `port``.expectedtext.Visitor::expectingreceives no value, so it cannot quote one.serde::de::Error::custommessages count as values and are dropped.Serializedso a diagnostic names the layerdefaultsinstead of the Rust type it was handed, and so keys render dotted instead of prefixed with the profile they were found under.LoaderimplementsDebugby hand and lists the layer names. Figment's derivedDebugrenders the merged values, which would put configuration values into any log line that formats a loader.lib.rsfor theLoader,error.rsfor the error type and the diagnostic translation, anddefaults.rsfor the provider. Each later source is a provider plus aLoadermethod and gets a module of its own.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
turbo.json's have been updated to reflect thisSerde stops at the first error within a single document, so a configuration with three mistakes reports them one run at a time. More than one diagnostic appears only when several sources each fail to produce data at all. This is Serde's behaviour rather than something the loader chooses, and reporting every mistake in one pass would mean writing a deserializer of our own.
LoadErrorhas a single variant, so a caller can render a report but cannot branch on what went wrong; the categories live in a private type. That is enough while there is one failure mode. Adding variants is a breaking change for callers, and doing it now would mean guessing at the joints before the file, environment and override layers exist.unknown fieldonly fires for configuration types that set#[serde(deny_unknown_fields)]. Without it, Serde ignores an unrecognised key and the field takes its default, so a typo passes silently. Closing that properly needs the schema walk, which can compare a document against the schema whether or not the attribute is set.🐾 Next steps
The remaining slices under H-4170: file sources with profiles and the directory cascade, the environment layer,
-Coverrides with the clap integration, the schema walk, and theconfigsubcommand.set_default(key, value)for seeding a single subtree belongs to the defaults layer and is not in this PR.🛡 What tests cover this?
libs/@local/config/tests/defaults.rscovers the merge contract and one case per diagnostic category, each asserting both halves of the contract: the key appears, the value does not.libs/@local/config/examples/defaults.rsis the runnable example and carries two tests of its own.test = trueon the[[example]]target is what makes them run, anddoc-scrape-examples = trueputs the example on theLoader::new,with_defaultsandloaddoc pages.Loader.❓ How to test this?
cargo nextest run --all-features --package hash-configcargo run --all-features --package hash-config --example defaults📹 Demo
The example composes two default layers and then sets the port to a password: