refactor: improve release handler code organization and reduce duplication#305
Merged
Conversation
- Convert all step-related error variants from tuple String to struct with message and source fields using BoxedStepError (Box<dyn Error>) - Add module-level documentation explaining the boxed error design decision - Replace generic Deployment/DeploymentFailed variants with specific: TrackerConfigDeployment, GrafanaProvisioningDeployment, PrometheusConfigDeployment, ComposeFilesDeployment - Update Traceable impl, error_kind(), and help() for new variants - Update handler.rs to construct errors with both message and source - Update tests to use struct variant syntax with helper function Trade-off: Using Box<dyn Error> loses pattern matching on source type, but enables uniform error handling for 15+ heterogeneous step error types and preserves error messages for debugging.
Reorganized the release command handler by extracting step implementations into a dedicated steps/ subdirectory organized by service: - steps/tracker.rs: tracker service release steps - steps/prometheus.rs: prometheus service steps (optional) - steps/grafana.rs: grafana service steps (optional) - steps/mysql.rs: mysql service steps (optional) - steps/caddy.rs: caddy service steps (HTTPS) - steps/compose.rs: docker compose deployment (async) - steps/common.rs: shared utilities (ansible_client helper) Each module only exposes its release() function publicly, with individual step implementations remaining private. This improves code organization and encapsulation while reducing handler.rs from 1114 to 291 lines (~74%).
Moved execute_release_workflow to a new workflow.rs module, separating: - handler.rs: State management, persistence, error handling - workflow.rs: Orchestrates step execution in correct order This makes the workflow a stateless free function that coordinates step calls, while the handler focuses on state transitions and persistence.
- Use explicit imports for step modules (tracker, prometheus, etc.) - Remove redundant inline comments since code is self-explanatory - Simplify doc comment to avoid duplicating individual module docs - Inline return value for conciseness
The instance_ip is now obtained from the environment object within each step, rather than being passed as a parameter. This simplifies the function signatures and reduces coupling. - Remove instance_ip param from workflow::execute() - Remove instance_ip param from compose::release() and deploy_files_to_remote() - Update compose logging to use environment.instance_ip() - Keep instance_ip validation in handler for early error detection
Move the conditional checks for optional services (MySQL, Caddy, Prometheus, Grafana) from individual step functions to the main release() function in each module. This eliminates repetitive checks in every step and makes the control flow clearer. - mysql.rs: Check uses_mysql() once in release() - caddy.rs: Check https().is_none() once in release() - prometheus.rs: Check prometheus().is_none() once in release() - grafana.rs: Check grafana().is_none() in release(), with secondary prometheus check after storage creation (since only provisioning requires Prometheus for datasource)
The analysis document was used during the refactoring process and is no longer needed now that the improvements have been implemented.
Member
Author
|
ACK 45588ce |
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
1 participant
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
This PR refactors the release command handler to improve code organization, reduce duplication, and enhance maintainability.
Changes
Module Reorganization
steps/:tracker.rs- Tracker service release stepsprometheus.rs- Prometheus service release stepsgrafana.rs- Grafana service release stepsmysql.rs- MySQL service release stepscaddy.rs- Caddy service release stepscompose.rs- Docker Compose deployment stepscommon.rs- Shared utilities (ansible client factory)Workflow Extraction
workflow.rs) - Separates orchestration logic from handler state managementCode Quality Improvements
instance_ipparameter from workflow - The IP is now obtained from the environment object within each step, reducing couplingrelease()functions - Eliminates repetitive conditional checks in every step function (~90 lines removed)Benefits
release()function in each module shows the complete service workflowTesting