-
Notifications
You must be signed in to change notification settings - Fork 23
Implement Admin APIs for partitioning #208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@luoyuxia @fresh-borzoni Would appreciate your reviews here! |
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.
Pull request overview
Implements client-side Admin APIs and supporting types/RPC plumbing for listing, creating, and dropping table partitions (closes #207).
Changes:
- Added partition metadata models (
PartitionSpec,PartitionInfo, etc.) and exported them viametadata. - Wired new Admin RPC requests/messages + API keys for
ListPartitionInfos,CreatePartition, andDropPartition. - Added an integration test covering the new partition Admin APIs.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/fluss/tests/integration/admin.rs | Adds integration coverage for partition list/create/drop APIs. |
| crates/fluss/src/client/admin.rs | Exposes partition Admin APIs on FlussAdmin. |
| crates/fluss/src/metadata/mod.rs | Registers and re-exports new partition metadata module. |
| crates/fluss/src/metadata/partition.rs | Introduces partition-related metadata types and unit tests. |
| crates/fluss/src/metadata/table.rs | Extends PhysicalTablePath to include an optional partition name and Display formatting. |
| crates/fluss/src/proto/fluss_api.proto | Adds protobuf messages for partition spec/info and partition Admin RPCs. |
| crates/fluss/src/rpc/api_key.rs | Adds API keys and numeric mappings for the new partition Admin RPCs. |
| crates/fluss/src/rpc/message/mod.rs | Registers and re-exports new partition RPC message modules. |
| crates/fluss/src/rpc/message/list_partition_infos.rs | Implements request/response plumbing for listing partitions (with optional spec filter). |
| crates/fluss/src/rpc/message/create_partition.rs | Implements request/response plumbing for creating partitions. |
| crates/fluss/src/rpc/message/drop_partition.rs | Implements request/response plumbing for dropping partitions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
@leekeiabstraction Thanks for the pr. LGTM! But copilot left minor comments, please have a look and decide to take copilot comment or not. Once you fix them, I'll merge.
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.
@leekeiabstraction Thanks for the PR.
Left comment
|
@fresh-borzoni @luoyuxia Thank you for the reviews. I've addressed all copilot comments. PTAL! |
fresh-borzoni
left a comment
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.
@leekeiabstraction Thank you, left another comments, I think we have some leftovers
PTAL
|
@fresh-borzoni TY for the comments. Addressed them, PTAL! |
fresh-borzoni
left a comment
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.
@leekeiabstraction Thank you, LGTM
luoyuxia
left a comment
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.
@leekeiabstraction Thnaks. LGTM!
Purpose
Linked issue: close #207
Brief change log
Tests
Added UTs and IT