gnuhpc commented on PR #629:
URL: https://github.com/apache/fluss-rust/pull/629#issuecomment-4753685507

   Addressed the `proto::*`-in-public-API feedback in 720d069. Summary of what 
changed:
   
   **Domain types introduced** under `crates/fluss/src/metadata/` (mirroring 
Java with `to_pb` / `from_pb` conversion in the wrapper layer):
   
   - `config.rs` — `AlterConfig` + `AlterConfigOpType` 
(Set/Delete/Append/Subtract)
   - `acl.rs` — `AclInfo` / `AclFilter` + `ResourceType` 
(Any/Cluster/Database/Table), `OperationType` 
(Any/All/Read/Write/Create/Drop/Alter/Describe), `PermissionType` (Any/Allow)
   - `table_change.rs` — `AddColumn` / `DropColumn` / `RenameColumn` / 
`ModifyColumn` + `ColumnPositionType` (currently `Last` only, matching Java's 
`ColumnPositionType.java` — proto reserves `First=1`, `After=3` for future use)
   - `table_stats.rs` — `BucketStatsRequest`
   
   Enum int codes are taken verbatim from the Java source 
(`AlterConfigOpType.java`, `ResourceType.java`, `OperationType.java`, 
`PermissionType.java`, `ColumnPositionType.java`).
   
   **Wrapper changes**: all 7 arg-taking wrappers' `new()` now accept domain 
types and call `.to_pb()` to populate `inner_request`. Wrappers still hold 
`proto::*` internally (matches the existing 
`PartitionSpec`/`DatabaseDescriptor` pattern).
   
   **Stack-wide sweep**: since the same `proto::*`-leak class of issue exists 
on #630 and #631 (the FlussAdmin admin client surface in #631 was where most 
leaks lived), I'm restacking the rest of the chain with the same cleanup now — 
will post here once each PR is force-pushed.
   
   **Tests**: added `*_pb_roundtrip` unit tests for each new domain type 
(matching the `PartitionSpec`/`DatabaseDescriptor` test style). `cargo test -p 
fluss-rs --lib` passes 544 (15 new).
   
   Thanks for the review!


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to