chore: remove unused devtools and rest_api crates - #419
Merged
Merged
Conversation
- Remove src/devtools crate and all its files - Remove src/rest_api crate and all its files - Update Cargo.toml workspace members - Remove proxy-related components from cluster_mgr - Update build scripts accordingly
|
Warning Review limit reached
More reviews will be available in 22 minutes and 38 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
📝 WalkthroughWalkthroughThis PR removes two crates ( ChangesWorkspace restructuring and feature removal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Removes unused workspace crates (devtools, rest_api) and strips proxy-related functionality from cluster_mgr, simplifying the workspace to a single crate (cluster_mgr) and updating packaging scripts accordingly.
Changes:
- Removed
src/devtoolsandsrc/rest_apicrates (code + configs). - Removed proxy config/state/tasking/CLI wiring from
cluster_mgr. - Updated workspace membership and release tarball build script to only build/package
cluster_mgr.
Reviewed changes
Copilot reviewed 69 out of 70 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
Cargo.toml |
Drops workspace members down to only src/cluster_mgr. |
scripts/build-release-tarball.sh |
Stops building/packaging the removed rest_api binary. |
src/rest_api/Cargo.toml |
Removed REST API crate manifest. |
src/rest_api/src/global_handler.rs |
Removed REST API crate code (global handler). |
src/rest_api/src/handler.rs |
Removed REST API crate code (HTTP handlers). |
src/rest_api/src/lib.rs |
Removed REST API crate code (types + signal handling). |
src/rest_api/src/main.rs |
Removed REST API crate binary entrypoint. |
src/rest_api/src/server.rs |
Removed REST API server implementation. |
src/devtools/Cargo.toml |
Removed devtools crate manifest. |
src/devtools/src/lib.rs |
Removed devtools crate library module declarations. |
src/devtools/src/main.rs |
Removed devtools crate binary entrypoint. |
src/devtools/src/term.rs |
Removed devtools interactive CLI implementation. |
src/devtools/src/config/common.rs |
Removed devtools config model definitions. |
src/devtools/src/config/config_macro.rs |
Removed devtools config/build macros. |
src/devtools/src/config/mod.rs |
Removed devtools config loader and helpers. |
src/devtools/src/cmd/base.rs |
Removed devtools command execution framework. |
src/devtools/src/cmd/check_mysql_status.rs |
Removed devtools MySQL process checks. |
src/devtools/src/cmd/check_user_exists.rs |
Removed devtools DB user existence checks. |
src/devtools/src/cmd/cmd_const.rs |
Removed devtools command/constants. |
src/devtools/src/cmd/cmd_macro.rs |
Removed devtools command macro implementations. |
src/devtools/src/cmd/cmd_runner.rs |
Removed devtools command dispatcher. |
src/devtools/src/cmd/cmd_utils.rs |
Removed devtools utility helpers (progress, process, storage checks). |
src/devtools/src/cmd/ctl_mysql_process.rs |
Removed devtools MySQL start/stop logic. |
src/devtools/src/cmd/download_third_party.rs |
Removed devtools third-party download logic. |
src/devtools/src/cmd/exec_init_sql_script.rs |
Removed devtools SQL initialization runner. |
src/devtools/src/cmd/gen_mysql_cnf.rs |
Removed devtools MySQL config generation. |
src/devtools/src/cmd/git_clone_source.rs |
Removed devtools git clone helpers. |
src/devtools/src/cmd/init_db.rs |
Removed devtools DB init flow. |
src/devtools/src/cmd/install_deps.rs |
Removed devtools dependency installation flow. |
src/devtools/src/cmd/install_dev_deps.rs |
Removed devtools dev dependency installation flow. |
src/devtools/src/cmd/mod.rs |
Removed devtools command module exports. |
src/devtools/src/cmd/mysql_ctl_util.rs |
Removed devtools MySQL CLI helper utilities. |
src/devtools/src/cmd/playground.rs |
Removed devtools playground command. |
src/devtools/src/cmd/setup_workspace.rs |
Removed devtools workspace setup command. |
src/devtools/config/cassandra.toml |
Removed devtools sample cassandra config. |
src/devtools/config/common-template.toml |
Removed devtools config template. |
src/devtools/config/common.toml |
Removed devtools sample common config. |
src/devtools/config/deps/darwin |
Removed devtools dependency list (darwin). |
src/devtools/config/deps/ubuntu |
Removed devtools dependency list (ubuntu). |
src/devtools/config/mysql/init.sql |
Removed devtools sample SQL. |
src/devtools/config/mysql/mysql_template.cnf |
Removed devtools MySQL template config. |
src/cluster_mgr/config/deployment_sqlite.sql |
Drops the t_proxy table from the sqlite schema. |
src/cluster_mgr/src/state/mod.rs |
Removes proxy_operation module export. |
src/cluster_mgr/src/state/proxy_operation.rs |
Removes proxy state operation implementation. |
src/cluster_mgr/src/state/state_mgr.rs |
Removes proxy state wiring and proxy state accessors. |
src/cluster_mgr/src/config/mod.rs |
Removes proxy-related config modules/types and storage provider enum entries. |
src/cluster_mgr/src/config/proxy_config_base.rs |
Removes proxy YAML config loading/generation. |
src/cluster_mgr/src/config/proxy_service.rs |
Removes proxy service config struct. |
src/cluster_mgr/src/config/storage_service_config.rs |
Adjusts storage provider detection logic (notably DynamoDB branch removal). |
src/cluster_mgr/src/config/deployment.rs |
Removes now-unreachable proxy/dynamodb-specific branches. |
src/cluster_mgr/src/config/config_base.rs |
Removes proxy package handling from deploy config unpacking. |
src/cluster_mgr/src/cli/mod.rs |
Removes eloqctl proxy ... CLI subcommand surface. |
src/cluster_mgr/src/cli/cmd_base.rs |
Removes proxy command execution/config persistence paths. |
src/cluster_mgr/src/cli/task/mod.rs |
Stops compiling proxy task module. |
src/cluster_mgr/src/cli/task/proxy_ctl_task.rs |
Removes proxy control task implementation. |
src/cluster_mgr/src/cli/task/task_controller.rs |
Removes proxy config naming branch. |
src/cluster_mgr/src/cli/task/check_task.rs |
Removes proxy check execution path. |
src/cluster_mgr/src/cli/task/download_task.rs |
Removes proxy-binary download helper path. |
src/cluster_mgr/src/cli/task/eloq_store_data_clean_task.rs |
Removes proxy config handling branch in task creation. |
src/cluster_mgr/src/cli/task/upload/mod.rs |
Stops compiling proxy upload builder module. |
src/cluster_mgr/src/cli/task/upload/proxy_upload_builder.rs |
Removes proxy upload builder implementation. |
src/cluster_mgr/src/cli/task/upload/upload_task_builder.rs |
Removes proxy upload builder type/dispatch. |
src/cluster_mgr/src/cli/task/upload/tx_conf_upload_builder.rs |
Adjusts scale-node upload logic (introduced a non-exhaustive match). |
src/cluster_mgr/src/cli/task/group/mod.rs |
Removes proxy task group/config variant wiring. |
src/cluster_mgr/src/cli/task/group/proxy_ctl_group.rs |
Removes proxy task group implementation. |
src/cluster_mgr/src/cli/task/group/remove_group.rs |
Removes DynamoDB removal branch and simplifies provider matching. |
src/cluster_mgr/src/cli/task/group/update_config_group.rs |
Removes unreachable fallback (introduced a non-exhaustive match). |
src/cluster_mgr/src/cli/task/group/monitor_ctl_group.rs |
Removes unreachable fallback (introduced a non-exhaustive match). |
src/cluster_mgr/src/cli/task/group/log_srv_ctl_group.rs |
Removes unreachable fallback (introduced a non-exhaustive match). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
550
to
553
| // Get the candidate status for this node from the is_candidate parameter | ||
| let node_is_candidate = match is_candidate { | ||
| Some(candidate_list) => candidate_list[i], | ||
| _ => unreachable!(), | ||
| }; |
Comment on lines
38
to
41
| password, | ||
| .. | ||
| } => (*restart, fields.clone(), *tx_node_id, password), | ||
| _ => unreachable!(), | ||
| }; |
Comment on lines
45
to
47
| let monitor_ctl_cmd = match &cmd_arg { | ||
| SubCommand::Monitor { command, .. } => command, | ||
| _ => unreachable!(), | ||
| }; |
Comment on lines
24
to
28
| let log_ctl_cmd_name = match cmd_arg.clone() { | ||
| SubCommand::LogService { | ||
| cluster: _, | ||
| command: log_ctl_cmd, | ||
| } => log_ctl_cmd, |
Comment on lines
115
to
119
| impl StorageService { | ||
| pub fn provider(&self) -> Option<StorageProvider> { | ||
| if self.dynamodb.is_some() { | ||
| Some(StorageProvider::Dynamodb) | ||
| } else if self.rocksdb.is_some() { | ||
| if self.rocksdb.is_some() { | ||
| Some(StorageProvider::Rocksdb) | ||
| } else if self.eloqdss.is_some() { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cluster_mgr/src/config/storage_service_config.rs (1)
116-124:⚠️ Potential issue | 🟠 MajorReject unsupported DynamoDB configs to prevent provider().unwrap panics
StorageProviderhas noDynamodbvariant, andStorageService::provider()only mapsrocksdb/eloqdss—so whendeployment.storage_service.dynamodbis set (withoutrocksdb/eloqdss),provider()returnsNone. However,validate_topology()still treatsdynamodbas a valid provider (it only enforces “exactly one is set”), and later code callsstorage.provider().unwrap()(e.g.,deployment.rs) /self.provider().unwrap()(viapretty_name()), which will panic.Consider either:
- Remove
Dynamodb/dynamodbfromStorageService(and related config/CLI fields) if DynamoDB is no longer supported.- Add a validation error in
validate_topology()(or before any unwrap) whenstorage_service.dynamodbis configured, explaining DynamoDB is deprecated/unsupported.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cluster_mgr/src/config/storage_service_config.rs` around lines 116 - 124, The provider() function only returns Rocksdb/EloqDSS and will return None for a configured dynamodb, causing .unwrap() panics (e.g., in deployment.rs and pretty_name()); add an explicit validation in validate_topology() (or the topology validation path) that rejects any StorageService with dynamodb set and returns a clear validation error like "DynamoDB is unsupported/deprecated" so deployments with storage_service.dynamodb fail early; reference StorageService.dynamodb, StorageProvider, provider(), validate_topology(), pretty_name(), and deployment.rs when implementing the new validation to prevent later provider().unwrap() panics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/cluster_mgr/src/config/storage_service_config.rs`:
- Around line 116-124: The provider() function only returns Rocksdb/EloqDSS and
will return None for a configured dynamodb, causing .unwrap() panics (e.g., in
deployment.rs and pretty_name()); add an explicit validation in
validate_topology() (or the topology validation path) that rejects any
StorageService with dynamodb set and returns a clear validation error like
"DynamoDB is unsupported/deprecated" so deployments with
storage_service.dynamodb fail early; reference StorageService.dynamodb,
StorageProvider, provider(), validate_topology(), pretty_name(), and
deployment.rs when implementing the new validation to prevent later
provider().unwrap() panics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f064d568-dafe-4c5a-94b8-ff59dc4d2be7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (69)
Cargo.tomlscripts/build-release-tarball.shsrc/cluster_mgr/config/deployment_sqlite.sqlsrc/cluster_mgr/src/cli/cmd_base.rssrc/cluster_mgr/src/cli/mod.rssrc/cluster_mgr/src/cli/task/check_task.rssrc/cluster_mgr/src/cli/task/download_task.rssrc/cluster_mgr/src/cli/task/eloq_store_data_clean_task.rssrc/cluster_mgr/src/cli/task/group/log_srv_ctl_group.rssrc/cluster_mgr/src/cli/task/group/mod.rssrc/cluster_mgr/src/cli/task/group/monitor_ctl_group.rssrc/cluster_mgr/src/cli/task/group/proxy_ctl_group.rssrc/cluster_mgr/src/cli/task/group/remove_group.rssrc/cluster_mgr/src/cli/task/group/update_config_group.rssrc/cluster_mgr/src/cli/task/mod.rssrc/cluster_mgr/src/cli/task/proxy_ctl_task.rssrc/cluster_mgr/src/cli/task/task_controller.rssrc/cluster_mgr/src/cli/task/upload/mod.rssrc/cluster_mgr/src/cli/task/upload/proxy_upload_builder.rssrc/cluster_mgr/src/cli/task/upload/tx_conf_upload_builder.rssrc/cluster_mgr/src/cli/task/upload/upload_task_builder.rssrc/cluster_mgr/src/config/config_base.rssrc/cluster_mgr/src/config/deployment.rssrc/cluster_mgr/src/config/mod.rssrc/cluster_mgr/src/config/proxy_config_base.rssrc/cluster_mgr/src/config/proxy_service.rssrc/cluster_mgr/src/config/storage_service_config.rssrc/cluster_mgr/src/state/mod.rssrc/cluster_mgr/src/state/proxy_operation.rssrc/cluster_mgr/src/state/state_mgr.rssrc/devtools/Cargo.tomlsrc/devtools/config/cassandra.tomlsrc/devtools/config/common-template.tomlsrc/devtools/config/common.tomlsrc/devtools/config/deps/darwinsrc/devtools/config/deps/ubuntusrc/devtools/config/mysql/init.sqlsrc/devtools/config/mysql/mysql_template.cnfsrc/devtools/src/cmd/base.rssrc/devtools/src/cmd/check_mysql_status.rssrc/devtools/src/cmd/check_user_exists.rssrc/devtools/src/cmd/cmd_const.rssrc/devtools/src/cmd/cmd_macro.rssrc/devtools/src/cmd/cmd_runner.rssrc/devtools/src/cmd/cmd_utils.rssrc/devtools/src/cmd/ctl_mysql_process.rssrc/devtools/src/cmd/download_third_party.rssrc/devtools/src/cmd/exec_init_sql_script.rssrc/devtools/src/cmd/gen_mysql_cnf.rssrc/devtools/src/cmd/git_clone_source.rssrc/devtools/src/cmd/init_db.rssrc/devtools/src/cmd/install_deps.rssrc/devtools/src/cmd/install_dev_deps.rssrc/devtools/src/cmd/mod.rssrc/devtools/src/cmd/mysql_ctl_util.rssrc/devtools/src/cmd/playground.rssrc/devtools/src/cmd/setup_workspace.rssrc/devtools/src/config/common.rssrc/devtools/src/config/config_macro.rssrc/devtools/src/config/mod.rssrc/devtools/src/lib.rssrc/devtools/src/main.rssrc/devtools/src/term.rssrc/rest_api/Cargo.tomlsrc/rest_api/src/global_handler.rssrc/rest_api/src/handler.rssrc/rest_api/src/lib.rssrc/rest_api/src/main.rssrc/rest_api/src/server.rs
💤 Files with no reviewable changes (62)
- src/devtools/src/cmd/mod.rs
- src/devtools/config/common.toml
- src/cluster_mgr/src/state/proxy_operation.rs
- src/cluster_mgr/src/cli/task/upload/mod.rs
- src/devtools/config/cassandra.toml
- src/cluster_mgr/src/config/proxy_config_base.rs
- src/devtools/src/cmd/install_deps.rs
- src/devtools/src/lib.rs
- src/devtools/Cargo.toml
- src/devtools/src/cmd/git_clone_source.rs
- src/rest_api/Cargo.toml
- src/devtools/src/cmd/gen_mysql_cnf.rs
- src/devtools/config/deps/darwin
- src/cluster_mgr/src/cli/task/upload/tx_conf_upload_builder.rs
- src/devtools/src/term.rs
- src/cluster_mgr/src/cli/task/mod.rs
- src/devtools/config/common-template.toml
- src/devtools/src/main.rs
- src/devtools/src/config/config_macro.rs
- src/devtools/src/cmd/playground.rs
- src/cluster_mgr/src/cli/task/upload/proxy_upload_builder.rs
- src/cluster_mgr/src/config/proxy_service.rs
- src/cluster_mgr/src/cli/task/eloq_store_data_clean_task.rs
- src/devtools/src/cmd/mysql_ctl_util.rs
- src/devtools/src/cmd/install_dev_deps.rs
- src/rest_api/src/lib.rs
- src/devtools/src/cmd/setup_workspace.rs
- src/devtools/config/mysql/mysql_template.cnf
- src/cluster_mgr/src/cli/task/group/proxy_ctl_group.rs
- src/devtools/src/cmd/cmd_const.rs
- src/devtools/src/cmd/ctl_mysql_process.rs
- src/devtools/src/config/common.rs
- src/devtools/src/cmd/check_mysql_status.rs
- src/devtools/src/cmd/init_db.rs
- src/rest_api/src/handler.rs
- src/devtools/config/deps/ubuntu
- src/rest_api/src/global_handler.rs
- src/cluster_mgr/src/cli/mod.rs
- src/cluster_mgr/src/config/config_base.rs
- src/cluster_mgr/src/cli/task/check_task.rs
- src/cluster_mgr/src/cli/task/task_controller.rs
- src/devtools/src/cmd/exec_init_sql_script.rs
- src/rest_api/src/server.rs
- src/cluster_mgr/src/state/state_mgr.rs
- src/devtools/src/cmd/download_third_party.rs
- src/devtools/src/cmd/cmd_macro.rs
- src/devtools/src/cmd/base.rs
- src/cluster_mgr/src/cli/task/group/mod.rs
- src/cluster_mgr/src/cli/task/group/log_srv_ctl_group.rs
- src/devtools/src/cmd/cmd_runner.rs
- src/rest_api/src/main.rs
- src/devtools/config/mysql/init.sql
- src/cluster_mgr/src/config/mod.rs
- src/devtools/src/cmd/check_user_exists.rs
- src/devtools/src/config/mod.rs
- src/cluster_mgr/src/state/mod.rs
- src/cluster_mgr/src/cli/task/proxy_ctl_task.rs
- src/devtools/src/cmd/cmd_utils.rs
- src/cluster_mgr/src/config/deployment.rs
- src/cluster_mgr/config/deployment_sqlite.sql
- src/cluster_mgr/src/cli/task/group/monitor_ctl_group.rs
- src/cluster_mgr/src/cli/task/group/update_config_group.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remove unused crates and proxy-related components from the codebase:
src/devtoolscrate and all its filessrc/rest_apicrate and all its filesCargo.tomlworkspace members to only includesrc/cluster_mgrcluster_mgr:proxy_ctl_group.rsproxy_ctl_task.rsproxy_upload_builder.rsproxy_config_base.rsproxy_service.rsproxy_operation.rsSummary by CodeRabbit