hmon: fix Cargo build, small code changes#90
hmon: fix Cargo build, small code changes#90arkjedrz wants to merge 2 commits intoeclipse-score:mainfrom
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
There was a problem hiding this comment.
Pull request overview
This PR refactors the supervisor API client implementations into a separate module with proper feature flag support, fixes Cargo build configuration, and improves test reliability.
Changes:
- Extracted
SupervisorAPIClienttrait and implementations (StubSupervisorAPIClient,ScoreSupervisorAPIClient) fromworker.rsinto a newsupervisor_api_clientmodule with feature-flag-based selection - Fixed Cargo build by making
monitor_rsdependency optional and adding mutually exclusive feature flags (stub_supervisor_api_clientandscore_supervisor_api_client) - Replaced brittle hash value test in
tag.rswith robust equality/inequality tests
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/health_monitoring_lib/rust/worker.rs |
Removed supervisor API client implementations, updated imports to use new module, simplified Duration usage, made Checks enum pub(crate) for cross-module access |
src/health_monitoring_lib/rust/supervisor_api_client/mod.rs |
New module defining SupervisorAPIClient trait with compile-time feature flag enforcement for mutual exclusivity |
src/health_monitoring_lib/rust/supervisor_api_client/stub_supervisor_api_client.rs |
Stub implementation moved from worker.rs, used for testing |
src/health_monitoring_lib/rust/supervisor_api_client/score_supervisor_api_client.rs |
Score implementation moved from worker.rs, uses monitor_rs for production |
src/health_monitoring_lib/rust/lib.rs |
Updated to use SupervisorAPIClientImpl type alias from supervisor_api_client module |
src/health_monitoring_lib/rust/tag.rs |
Fixed brittle test that checked specific hash value; replaced with equality/inequality tests |
src/health_monitoring_lib/Cargo.toml |
Made monitor_rs optional, added feature flags with stub as default, removed extra blank line |
src/health_monitoring_lib/BUILD |
Added crate_features to rust targets to select appropriate supervisor API client |
Cargo.toml |
Added default-members to focus on health_monitoring_lib for development |
.vscode/settings.json |
Added rust-analyzer cargo features configuration for stub_supervisor_api_client |
.github/workflows/lint_clippy.yml |
Updated to test both feature flags independently |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The created documentation from the pull request is available at: docu-html |
| all(feature = "score_supervisor_api_client", feature = "stub_supervisor_api_client"), | ||
| not(any(feature = "score_supervisor_api_client", feature = "stub_supervisor_api_client")) | ||
| ))] | ||
| compile_error!("Either 'score_supervisor_api_client' or 'stub_supervisor_api_client' must be enabled!"); |
There was a problem hiding this comment.
Why is this needed here? There may be two implementations functionaning in system. Currently the only limiation is that you cannot inject them in start of health monitor but other than that, nothing prevents to impement N of them or ?
- Fix Cargo for `health_monitoring_lib`. - Fix leftover test changes from previous PR. - Small code changes.
3d21382 to
3cb438a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/health_monitoring_lib/rust/supervisor_api_client/score_supervisor_api_client.rs
Show resolved
Hide resolved
| #[cfg(feature = "score_supervisor_api_client")] | ||
| pub mod score_supervisor_api_client; | ||
| #[cfg(feature = "stub_supervisor_api_client")] | ||
| pub mod stub_supervisor_api_client; |
There was a problem hiding this comment.
There’s currently no compile-time guard to prevent enabling both score_supervisor_api_client and stub_supervisor_api_client (or neither). Adding compile_error! checks for these invalid feature combinations would prevent surprising runtime selection and avoid Cargo configurations that otherwise fail to compile.
- Less restrictive feature flags. - Additional small code changes.
9a0b549 to
35c7c0d
Compare
health_monitoring_lib.