Skip to content

hmon: fix Cargo build, small code changes#90

Merged
pawelrutkaq merged 3 commits intoeclipse-score:mainfrom
qorix-group:arkjedrz_fix-cargo-build
Mar 2, 2026
Merged

hmon: fix Cargo build, small code changes#90
pawelrutkaq merged 3 commits intoeclipse-score:mainfrom
qorix-group:arkjedrz_fix-cargo-build

Conversation

@arkjedrz
Copy link
Contributor

  • Fix Cargo for health_monitoring_lib.
  • Fix leftover test changes from previous PR.
  • Small code changes.

@github-actions
Copy link

github-actions bot commented Feb 24, 2026

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output
[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.4.2) and connecting to it...
INFO: Invocation ID: 9ff9b52e-eee5-4d50-8aff-5b785855c89a
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'score_rust_policies', the root module requires module version score_rust_policies@0.0.3, but got score_rust_policies@0.0.5 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (35 packages loaded, 9 targets configured)

Analyzing: target //:license-check (90 packages loaded, 16 targets configured)

Analyzing: target //:license-check (140 packages loaded, 2614 targets configured)

Analyzing: target //:license-check (148 packages loaded, 5608 targets configured)

Analyzing: target //:license-check (160 packages loaded, 7029 targets configured)

Analyzing: target //:license-check (160 packages loaded, 7817 targets configured)

Analyzing: target //:license-check (160 packages loaded, 7817 targets configured)

Analyzing: target //:license-check (160 packages loaded, 7817 targets configured)

Analyzing: target //:license-check (161 packages loaded, 7941 targets configured)

INFO: Analyzed target //:license-check (165 packages loaded, 9955 targets configured).
[5 / 14] checking cached actions
[13 / 16] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 0s disk-cache, processwrapper-sandbox
[14 / 16] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar; 0s disk-cache, processwrapper-sandbox
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 24.611s, Critical Path: 2.74s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 SupervisorAPIClient trait and implementations (StubSupervisorAPIClient, ScoreSupervisorAPIClient) from worker.rs into a new supervisor_api_client module with feature-flag-based selection
  • Fixed Cargo build by making monitor_rs dependency optional and adding mutually exclusive feature flags (stub_supervisor_api_client and score_supervisor_api_client)
  • Replaced brittle hash value test in tag.rs with 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.

@github-actions
Copy link

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!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +23 to +26
#[cfg(feature = "score_supervisor_api_client")]
pub mod score_supervisor_api_client;
#[cfg(feature = "stub_supervisor_api_client")]
pub mod stub_supervisor_api_client;
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@arkjedrz arkjedrz force-pushed the arkjedrz_fix-cargo-build branch from 9a0b549 to 35c7c0d Compare February 25, 2026 12:59
@arkjedrz arkjedrz temporarily deployed to workflow-approval February 25, 2026 12:59 — with GitHub Actions Inactive
@arkjedrz arkjedrz temporarily deployed to workflow-approval February 25, 2026 12:59 — with GitHub Actions Inactive
@arkjedrz arkjedrz had a problem deploying to workflow-approval March 2, 2026 09:05 — with GitHub Actions Failure
@arkjedrz arkjedrz temporarily deployed to workflow-approval March 2, 2026 09:22 — with GitHub Actions Inactive
@arkjedrz arkjedrz temporarily deployed to workflow-approval March 2, 2026 09:30 — with GitHub Actions Inactive
arkjedrz added 2 commits March 2, 2026 10:34
- Fix Cargo for `health_monitoring_lib`.
- Fix leftover test changes from previous PR.
- Small code changes.
- Less restrictive feature flags.
- Additional small code changes.
@arkjedrz arkjedrz force-pushed the arkjedrz_fix-cargo-build branch from 514d19e to f0a291f Compare March 2, 2026 09:34
@arkjedrz arkjedrz temporarily deployed to workflow-approval March 2, 2026 09:34 — with GitHub Actions Inactive
@arkjedrz arkjedrz temporarily deployed to workflow-approval March 2, 2026 09:34 — with GitHub Actions Inactive
Use `score_supervisor_api_client` as a default.
@arkjedrz arkjedrz force-pushed the arkjedrz_fix-cargo-build branch from f0a291f to cf4250b Compare March 2, 2026 12:50
@arkjedrz arkjedrz temporarily deployed to workflow-approval March 2, 2026 12:50 — with GitHub Actions Inactive
@arkjedrz arkjedrz temporarily deployed to workflow-approval March 2, 2026 12:50 — with GitHub Actions Inactive
@pawelrutkaq pawelrutkaq merged commit 4119793 into eclipse-score:main Mar 2, 2026
16 checks passed
@arkjedrz arkjedrz deleted the arkjedrz_fix-cargo-build branch March 2, 2026 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants