apollo_config,apollo_node_config: make config fields with custom deserializers serialize symmetrically#14630
Draft
nimrod-starkware wants to merge 1 commit into
Conversation
This was referenced Jun 25, 2026
Contributor
Author
nimrod-starkware
left a comment
Contributor
Author
There was a problem hiding this comment.
why is it needed again??
@nimrod-starkware made 1 comment.
Reviewable status: 0 of 21 files reviewed, all discussions resolved.
ad01662 to
81e18a7
Compare
22984a6 to
407157e
Compare
This was referenced Jun 28, 2026
Open
407157e to
9b677ae
Compare
81e18a7 to
4614c66
Compare
9b677ae to
de0822e
Compare
de0822e to
e752ded
Compare
0797ffc to
402a491
Compare
e752ded to
2c485b4
Compare
…rializers serialize symmetrically
Several config-struct fields use #[serde(deserialize_with = ...)] for a string/scalar
wire format (comma-separated lists; Duration as millis/seconds/float-seconds) but derive
a mismatched Serialize (array, or {secs,nanos}), so serde_json::to_value(&config) does not
round-trip them. Add a matching serialize_with to each (~39 fields across 14 crates),
authoring the missing converters in apollo_config. This aligns the derived Serialize with
the wire/jsonnet form the deserializers expect, so the native-config harness and
startup-log presentation round-trip faithfully.
Guarded by two tests: a converter-level round-trip (apollo_config/converters_test) and a
config-level round-trip in apollo_node_config that serializes->deserializes the real
component config structs with non-default values, catching a mispaired or missing
serialize_with. Sensitive<T> fields are intentionally left asymmetric (serialize redacts;
symmetry would leak secrets) and excluded from the config-level guard.
Only the derived Serialize changes; the hand-written SerializeConfig::dump() and the
committed config_schema.json / app_configs are untouched (no up-to-date guard flips).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
402a491 to
3918577
Compare
2c485b4 to
f7f7322
Compare
This was referenced Jul 2, 2026
Draft
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.

Several config-struct fields use #[serde(deserialize_with = ...)] for a string/scalar
wire format (comma-separated lists; Duration as millis/seconds/float-seconds) but derive
a mismatched Serialize (array, or {secs,nanos}), so serde_json::to_value(&config) does not
round-trip them. Add a matching serialize_with to each (~39 fields across 14 crates),
authoring the missing converters in apollo_config. This aligns the derived Serialize with
the wire/jsonnet form the deserializers expect, so the native-config harness and
startup-log presentation round-trip faithfully.
Guarded by two tests: a converter-level round-trip (apollo_config/converters_test) and a
config-level round-trip in apollo_node_config that serializes->deserializes the real
component config structs with non-default values, catching a mispaired or missing
serialize_with. Sensitive fields are intentionally left asymmetric (serialize redacts;
symmetry would leak secrets) and excluded from the config-level guard.
Only the derived Serialize changes; the hand-written SerializeConfig::dump() and the
committed config_schema.json / app_configs are untouched (no up-to-date guard flips).
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com