[df] Snake-case the display string of DetailedRunMode - #5495
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 904cdca72d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| .enum_attribute( | ||
| "DetailedRunMode", | ||
| "#[strum(serialize_all = \"snake_case\")]", |
There was a problem hiding this comment.
Add migration notes for the lower-case SQL values
This changes existing partition_state.effective_mode values such as Leader and BecomingLeader to leader and becoming_leader, so saved SQL predicates and monitoring queries comparing the old literals will silently stop matching after an upgrade. The commit contains no release note describing the changed values or required query updates; add one under release-notes/unreleased/ as required for breaking behavioral changes.
AGENTS.md reference: AGENTS.md:L51-L51
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
A release note wouldn't hurt. But I don't think that people really depend on this.
DetailedRunMode didn't have a snake_case display so it was a bit awkward in partition_state table having two coloums with different casing. This PR fixes that. The table has been there since 1.7.X, but I assume we don't have comptability promises on the format of our df tables. Before: ``` ~/repos/restate/restate ❯❯❯ rcluster ctl -c r18-perf sql "select * from partition_state limit 1" ✘ 2 PARTITION_ID PLAIN_NODE_ID GEN_NODE_ID TARGET_MODE EFFECTIVE_MODE UPDATED_AT LEADER_EPOCH LEADER APPLIED_LOG_LSN LAST_RECORD_APPLIED_AT REPLAY_STATUS DURABLE_LOG_LSN ARCHIVED_LOG_LSN TARGET_TAIL_LSN APPLIED_RULE_BOOK_VERSION APPLIED_SCHEMA_VERSION ENABLED_FEATURES BROKEN_REASON ENABLED_STORAGE_FEATURES 0 N1 N1:9 leader Leader 2026-10-02T18:26:32.516Z 39 N1:9 1624009 2026-10-02T17:18:12.937Z active 1621726 1624010 3 [journal_v2, vqueues, [scoped-promises-and-state-tables ``` After: ``` ~/repos/restate/restate ❯❯❯ rcluster ctl sql "select * from partition_state limit 1" PARTITION_ID PLAIN_NODE_ID GEN_NODE_ID TARGET_MODE EFFECTIVE_MODE UPDATED_AT LEADER_EPOCH LEADER APPLIED_LOG_LSN LAST_RECORD_APPLIED_AT REPLAY_STATUS DURABLE_LOG_LSN ARCHIVED_LOG_LSN TARGET_TAIL_LSN APPLIED_RULE_BOOK_VERSION APPLIED_SCHEMA_VERSION ENABLED_FEATURES BROKEN_REASON ENABLED_STORAGE_FEATURES 0 N1 N1:2 leader leader 2026-10-02T18:27:15.831Z 2 N1:2 2 2026-10-02T18:24:07.883Z active 0 3 0 [journal_v2, vqueues, [scoped-promises-and-state-tables ``` This follows the same pattern about having the enums (e.g. RunMode, BrokenReason, ReplayStatus) have snake case display and then they get rendered with TitleCase in the CLI (e.g. `render_replay_status`)
tillrohrmann
left a comment
There was a problem hiding this comment.
Thanks for aligning the casing in our DF tables @MohamedBassem. LGTM. +1 for merging.
| ) | ||
| .enum_attribute( | ||
| "DetailedRunMode", | ||
| "#[strum(serialize_all = \"snake_case\")]", |
There was a problem hiding this comment.
A release note wouldn't hurt. But I don't think that people really depend on this.
DetailedRunMode didn't have a snake_case display so it was a bit awkward in partition_state table having two coloums with different casing. This PR fixes that. The table has been there since 1.7.X, but I assume we don't have comptability promises on the format of our df tables.
Before:
After:
This follows the same pattern about having the enums (e.g. RunMode, BrokenReason, ReplayStatus) have snake case display and then they get rendered with TitleCase in the CLI (e.g.
render_replay_status)