Skip to content

refactor(types): replace schemaMigrationPhase magic strings with an enum#65

Merged
passcod merged 1 commit into
mainfrom
operator-schema-migration-phase-enum
Jun 8, 2026
Merged

refactor(types): replace schemaMigrationPhase magic strings with an enum#65
passcod merged 1 commit into
mainfrom
operator-schema-migration-phase-enum

Conversation

@passcod

@passcod passcod commented Jun 7, 2026

Copy link
Copy Markdown
Member

🤖

Summary

Replace the `Option` shape of `PostgresPhysicalReplicaStatus.schemaMigrationPhase` with a proper enum:

```rust
pub enum SchemaMigrationPhase {
Active,
Complete,
Partial,
TimeoutSkipped,
Failed(String),
}
```

with a single `is_settled() -> bool` method used by the sweep gate.

Wire compatibility

The on-disk / on-API format is preserved exactly via a custom Serialize/Deserialize that delegates to Display/FromStr:

Variant Wire string
`Active` `active`
`Complete` `complete`
`Partial` `partial`
`TimeoutSkipped` `timeout-skipped`
`Failed(reason)` `failed: `

The CRD's schema for the field still declares it as a plain string (custom `JsonSchema` impl pointing at `type: string`). Existing replicas in the field don't need migration; new operator code parses their existing status strings into the enum on read.

Why

The sweep-gate deadlock that #64 fixed was a missed match arm — the old `matches!(phase, None | Some("complete") | Some("partial"))` allow-list silently broke when `timeout-skipped` was added as a new terminal phase. That's exactly the class of bug the type system can catch: if the gate were written against an exhaustive match on the enum, the new variant would have surfaced as a compile error.

This PR doesn't change behaviour — #64 already fixed the deadlock — but it makes the next variant addition fail at compile time instead of in production. The new `is_settled()` method is the single canonical predicate; adding a new variant forces the author to think about whether it's settled-state or not.

Shape

  • `types/replica.rs`: enum definition + Display/FromStr/Serialize/Deserialize/JsonSchema impls.
  • `controllers/replica.rs`: all set-sites use the enum directly inside `serde_json::json!` (custom Serialize handles the conversion); reader sites use `matches!` on the enum.
  • `tests/persistent_schemas.rs`: integration assertions compare against `SchemaMigrationPhase::Complete` instead of `"complete"`.

Tests

5 new unit tests in `types/replica.rs::tests`:

  • Round-trip for the four nullary variants (serde Serialize → Deserialize).
  • Round-trip for `Failed(reason)` preserving the free-form reason string.
  • Wire-format pinning: each variant's exact serialized string is asserted (so a future "rename to be more pythonic" PR breaks at test time, not at runtime against external consumers).
  • `FromStr` rejects unknown strings.
  • `is_settled()` returns true for every variant except `Active`.

Note

Stacks naturally with #64 but doesn't depend on it — refactor only, no behaviour change.

Introduce a SchemaMigrationPhase enum to replace the Option<String>
that has been growing magic-string call sites. Variants: Active,
Complete, Partial, TimeoutSkipped, Failed(String). The wire format is
preserved exactly (custom Serialize/Deserialize via Display/FromStr)
so existing replica status objects round-trip unchanged, and the CRD
field continues to declare a plain string via a custom JsonSchema
impl.

Motivation is the sweep-gate deadlock fix landed separately: the old
allow-list shape ("phase == complete | partial") silently broke when
the timeout-skipped phase was added, since the allow-list missed the
new variant. The replacement gate (is_settled()) is exhaustive over
the enum and the compiler now catches the next variant that forgets
to handle this — turning a runtime deadlock into a compile error.
@passcod passcod merged commit 735f85d into main Jun 8, 2026
18 checks passed
@passcod passcod deleted the operator-schema-migration-phase-enum branch June 8, 2026 03:36
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.

1 participant