You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "tmchow (via GitHub)" <gi...@apache.org> on 2026/03/30 07:46:15 UTC
[PR] fix(common): add serde rename_all to snapshot enums [iggy]
tmchow opened a new pull request, #3051:
URL: https://github.com/apache/iggy/pull/3051
## Summary
Adds `#[serde(rename_all = "snake_case")]` to `SystemSnapshotType` and `SnapshotCompression` enums in `core/common/src/types/snapshot/mod.rs`.
## Changes
These two enums were missing the serde rename attribute that all other enums in `core/common/src/types/` use. The `Display` and `FromStr` impls already produce snake_case strings (e.g., `filesystem_overview`, `process_list`), so serde serialization should match.
Pattern used by other enums in the same directory:
- `consumer_kind.rs:49` — `#[serde(rename_all = "snake_case")]`
- `user_status.rs:26` — `#[serde(rename_all = "snake_case")]`
- `identifier/mod.rs:53` — `#[serde(rename_all = "snake_case")]`
- `message/partitioning_kind.rs:28` — `#[serde(rename_all = "snake_case")]`
## Testing
`cargo fmt --all -- --check` passes. No JSON fixtures or config files reference these enum variants in PascalCase.
## AI Usage
This contribution was developed with AI assistance (Claude Code + Codex).
Fixes #2699
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(common): add serde aliases for snake_case deserialization on snapshot enums [iggy]
Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on PR #3051:
URL: https://github.com/apache/iggy/pull/3051#issuecomment-4156779508
Sorry, but we're not interested in being an LLM proxy (looking at you em dash).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(common): add serde aliases for snake_case deserialization on snapshot enums [iggy]
Posted by "tmchow (via GitHub)" <gi...@apache.org>.
tmchow commented on PR #3051:
URL: https://github.com/apache/iggy/pull/3051#issuecomment-4159560539
> Sorry, but we're not interested in being an LLM proxy (looking at you em dash).
lol, not a bot. I often use emdashes!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(common): add serde aliases for snake_case deserialization on snapshot enums [iggy]
Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz closed pull request #3051: fix(common): add serde aliases for snake_case deserialization on snapshot enums
URL: https://github.com/apache/iggy/pull/3051
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(common): add serde aliases for snake_case deserialization on snapshot enums [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on code in PR #3051:
URL: https://github.com/apache/iggy/pull/3051#discussion_r3010935905
##########
core/common/src/types/snapshot/mod.rs:
##########
@@ -35,36 +35,49 @@ impl Snapshot {
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub enum SystemSnapshotType {
/// Overview of the filesystem.
+ #[serde(alias = "filesystem_overview")]
Review Comment:
the `serde(alias)` approach creates a serialize/deserialize asymmetry - the server outputs PascalCase but accepts snake_case on input. a round-trip through `deserialize("filesystem_overview") -> serialize` produces `"FilesystemOverview"` - a different string. the wire format has no single source of truth.
the codebase standard is `#[serde(rename_all = "snake_case")]`, used by `UserStatus`, `ConsumerKind`, `IdKind`, `PartitioningKind`, `PollingKind`, `DiagnosticEvent`, `HeaderKind` - 7 enums total. these snapshot enums should use the same approach so serialize and deserialize are symmetric.
this doesn't fix #2699 which asks for a consistent serialization strategy - it introduces a new inconsistency instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(common): add serde rename_all to snapshot enums [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3051:
URL: https://github.com/apache/iggy/pull/3051#issuecomment-4153153784
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/3051?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 68.96%. Comparing base ([`169a09b`](https://app.codecov.io/gh/apache/iggy/commit/169a09bcd5c336de73e14d0350fe66f8f2968b7d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ded45ed`](https://app.codecov.io/gh/apache/iggy/commit/ded45ed7b5762ab2ab8f1c60d7721bf88bb3beff?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3051 +/- ##
============================================
- Coverage 71.77% 68.96% -2.82%
Complexity 943 943
============================================
Files 1121 1058 -63
Lines 93814 89007 -4807
Branches 71125 71037 -88
============================================
- Hits 67338 61386 -5952
- Misses 23842 25087 +1245
+ Partials 2634 2534 -100
```
| [Components](https://app.codecov.io/gh/apache/iggy/pull/3051/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [Rust Core](https://app.codecov.io/gh/apache/iggy/pull/3051/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.35% <ø> (ø)` | |
| [Java SDK](https://app.codecov.io/gh/apache/iggy/pull/3051/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.30% <ø> (ø)` | |
| [C# SDK](https://app.codecov.io/gh/apache/iggy/pull/3051/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.65% <ø> (ø)` | |
| [Python SDK](https://app.codecov.io/gh/apache/iggy/pull/3051/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.43% <ø> (ø)` | |
| [Node SDK](https://app.codecov.io/gh/apache/iggy/pull/3051/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `87.29% <ø> (-4.24%)` | :arrow_down: |
| [Go SDK](https://app.codecov.io/gh/apache/iggy/pull/3051/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `10.00% <ø> (-28.97%)` | :arrow_down: |
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/3051?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [core/common/src/types/snapshot/mod.rs](https://app.codecov.io/gh/apache/iggy/pull/3051?src=pr&el=tree&filepath=core%2Fcommon%2Fsrc%2Ftypes%2Fsnapshot%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb21tb24vc3JjL3R5cGVzL3NuYXBzaG90L21vZC5ycw==) | `58.51% <ø> (ø)` | |
... and [122 files with indirect coverage changes](https://app.codecov.io/gh/apache/iggy/pull/3051/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(common): add serde rename_all to snapshot enums [iggy]
Posted by "tmchow (via GitHub)" <gi...@apache.org>.
tmchow commented on PR #3051:
URL: https://github.com/apache/iggy/pull/3051#issuecomment-4155999080
Good catch, you're right — `rename_all = "snake_case"` changes the serialization output from PascalCase to snake_case, which breaks the HTTP API wire format that all foreign SDKs depend on.
Fixed by replacing `rename_all` with per-variant `#[serde(alias = "...")]` attributes. This preserves PascalCase in serialization output (so `HttpMessageStream.cs` and all other SDKs continue to work unchanged) while accepting snake_case during deserialization. The `Display`/`FromStr` impls already handle snake_case for the CLI path independently.
Checked the other enums in the codebase (`UserStatus`, `ConsumerKind`, etc.) that use `rename_all` — those don't have the same cross-SDK exposure since they're not used in the `/snapshot` HTTP endpoint the same way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org