Repository navigation
fix(inkless:config): allow routine alters on diskless topics with remote.storage.enable=false - #804
#804Conversation
2fdebe8 to
f10dcdc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Deleting an enabled remote-storage override can bypass the consolidation invariant.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Allows routine configuration updates for legacy diskless topics with persisted remote.storage.enable=false.
Changes:
- Grandfathers unchanged diskless/remote-disabled configurations.
- Prioritizes the consolidation-specific validation error.
- Updates unit and integration expectations.
File summaries
| File | Description |
|---|---|
storage/.../LogConfig.java |
Refines transition validation. |
core/.../LogConfigTest.scala |
Expands transition tests. |
core/.../DisklessAndRemoteStorageConfigsTest.java |
Updates integration assertions. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ote.storage.enable=false The controller validates the merged topic config map, so an existing remote.storage.enable=false override looked like a disable on every alter. Reject only new diskless + remote=false transitions, and do that before mutual exclusion so the specific invariant still fires. Co-authored-by: Cursor <cursoragent@cursor.com>
2e71ee1 to
3d356ce
Compare
A deleted remote.storage.enable override disappears from the resulting config map. Resolve the target state from the combined config so incremental deletion and legacy omission cannot leave a diskless topic without remote storage. Co-authored-by: Cursor <cursoragent@cursor.com>
3d356ce to
79b0ca3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟢 Approval recommended
The validation change and its regression coverage were fully reviewed with no unresolved issues.
Review effort: Balanced
Findings: None
Resolved since last review (1)
…ote.storage.enable=false (#804) * fix(inkless:config): allow routine alters on diskless topics with remote.storage.enable=false The controller validates the merged topic config map, so an existing remote.storage.enable=false override looked like a disable on every alter. Reject only new diskless + remote=false transitions, and do that before mutual exclusion so the specific invariant still fires. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:config): reject remote storage override deletion A deleted remote.storage.enable override disappears from the resulting config map. Resolve the target state from the combined config so incremental deletion and legacy omission cannot leave a diskless topic without remote storage. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…ote.storage.enable=false (#804) * fix(inkless:config): allow routine alters on diskless topics with remote.storage.enable=false The controller validates the merged topic config map, so an existing remote.storage.enable=false override looked like a disable on every alter. Reject only new diskless + remote=false transitions, and do that before mutual exclusion so the specific invariant still fires. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:config): reject remote storage override deletion A deleted remote.storage.enable override disappears from the resulting config map. Resolve the target state from the combined config so incremental deletion and legacy omission cannot leave a diskless topic without remote storage. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…ote.storage.enable=false (#804) * fix(inkless:config): allow routine alters on diskless topics with remote.storage.enable=false The controller validates the merged topic config map, so an existing remote.storage.enable=false override looked like a disable on every alter. Reject only new diskless + remote=false transitions, and do that before mutual exclusion so the specific invariant still fires. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:config): reject remote storage override deletion A deleted remote.storage.enable override disappears from the resulting config map. Resolve the target state from the combined config so incremental deletion and legacy omission cannot leave a diskless topic without remote storage. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>

The controller validates the merged topic config map, so an existing remote.storage.enable=false override looked like a disable on every alter. Reject only new diskless + remote=false transitions, and do that before mutual exclusion so the specific invariant still fires.