Skip to content

Conversation

@tyranron
Copy link
Contributor

@tyranron tyranron commented Feb 4, 2026

Complements #178
Related to #161

Synopsis

At the moment trying to deserialize Config into a struct with u128/i128 errors:

thread 'conf::conf_parse_spec::reads_from_yaml_file' (65705764) panicked at src/conf.rs:688:41:
Failed to parse config: u128 is not supported for key `db.tigerbeetle.cluster_id`

This happens even if the parsed value is small and fits into other integers.

Solution

This PR wires the Value's i128/u128 functionality into Deserializer/Serializer implementations.

@tyranron tyranron force-pushed the fix-parsing-into-i128 branch from 3ca2522 to f99d86e Compare February 4, 2026 13:41
@coveralls
Copy link

coveralls commented Feb 4, 2026

Pull Request Test Coverage Report for Build 21673872770

Details

  • 1 of 24 (4.17%) changed or added relevant lines in 3 files are covered.
  • 15 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-1.3%) to 62.809%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/value.rs 0 1 0.0%
src/de.rs 1 4 25.0%
src/ser.rs 0 19 0.0%
Files with Coverage Reduction New Missed Lines %
src/de.rs 7 84.66%
src/ser.rs 8 33.33%
Totals Coverage Status
Change from base Build 21554873549: -1.3%
Covered Lines: 939
Relevant Lines: 1495

💛 - Coveralls

@tyranron tyranron force-pushed the fix-parsing-into-i128 branch from f99d86e to e06fd8c Compare February 4, 2026 13:46
@tyranron tyranron force-pushed the fix-parsing-into-i128 branch from e06fd8c to 6981823 Compare February 4, 2026 13:47
@tyranron
Copy link
Contributor Author

tyranron commented Feb 4, 2026

CI error with failed coverage publishing is not relevant to this PR. Everything else made green.

Comment on lines +111 to +126
fn serialize_i128(self, v: i128) -> Result<Self::Ok> {
if v > i64::MAX.into() {
Err(ConfigError::Message(format!(
"value {} is greater than the max {}",
v,
i64::MAX
)))
} else if v < i64::MIN.into() {
Err(ConfigError::Message(format!(
"value {} is less than the min {}",
v,
i64::MIN
)))
} else {
self.serialize_i64(v as i64)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we checking the ranges rather than serializing as a primitive?

Copy link
Contributor Author

@tyranron tyranron Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@epage I don't know. I did repeat the logic that was already there for the serialize_u64(). It seems strange to me too, but I can only speculate that it's there for compatibility with JSON integer types, or something.

Personally, I don't see any value in it. But changing this would be a subtle breaking change, so seemed to be out of scope for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants