thin-edge / thin-edge.io

The open edge framework for lightweight IoT devices
https://thin-edge.io
Apache License 2.0
211 stars 55 forks source link

fix(mqtt): Map mosquitto bridge service health status updates #2903

Closed albinsuresh closed 1 month ago

albinsuresh commented 1 month ago

Proposed changes

Types of changes

Paste Link to the issue

https://github.com/thin-edge/thin-edge.io/issues/2902

Checklist

Further comments

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 94.28571% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 78.0%. Comparing base (8a91efb) to head (d52bbdf). Report is 2 commits behind head on main.

Additional details and impacted files | [Files](https://app.codecov.io/gh/thin-edge/thin-edge.io/pull/2903?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thin-edge) | Coverage Δ | | |---|---|---| | [crates/core/tedge\_api/src/health.rs](https://app.codecov.io/gh/thin-edge/thin-edge.io/pull/2903?src=pr&el=tree&filepath=crates%2Fcore%2Ftedge_api%2Fsrc%2Fhealth.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thin-edge#diff-Y3JhdGVzL2NvcmUvdGVkZ2VfYXBpL3NyYy9oZWFsdGgucnM=) | `92.6% <ø> (ø)` | | | [crates/core/tedge\_api/src/mqtt\_topics.rs](https://app.codecov.io/gh/thin-edge/thin-edge.io/pull/2903?src=pr&el=tree&filepath=crates%2Fcore%2Ftedge_api%2Fsrc%2Fmqtt_topics.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thin-edge#diff-Y3JhdGVzL2NvcmUvdGVkZ2VfYXBpL3NyYy9tcXR0X3RvcGljcy5ycw==) | `86.9% <100.0%> (-0.6%)` | :arrow_down: | | [...s/extensions/c8y\_mapper\_ext/src/service\_monitor.rs](https://app.codecov.io/gh/thin-edge/thin-edge.io/pull/2903?src=pr&el=tree&filepath=crates%2Fextensions%2Fc8y_mapper_ext%2Fsrc%2Fservice_monitor.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thin-edge#diff-Y3JhdGVzL2V4dGVuc2lvbnMvYzh5X21hcHBlcl9leHQvc3JjL3NlcnZpY2VfbW9uaXRvci5ycw==) | `97.6% <100.0%> (+1.2%)` | :arrow_up: | | [crates/extensions/c8y\_mapper\_ext/src/converter.rs](https://app.codecov.io/gh/thin-edge/thin-edge.io/pull/2903?src=pr&el=tree&filepath=crates%2Fextensions%2Fc8y_mapper_ext%2Fsrc%2Fconverter.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thin-edge#diff-Y3JhdGVzL2V4dGVuc2lvbnMvYzh5X21hcHBlcl9leHQvc3JjL2NvbnZlcnRlci5ycw==) | `83.6% <77.7%> (+<0.1%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/thin-edge/thin-edge.io/pull/2903/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thin-edge)
github-actions[bot] commented 1 month ago

Robot Results

:white_check_mark: Passed :x: Failed :next_track_button: Skipped Total Pass % :stopwatch: Duration
443 0 3 443 100 1h0m32.488258999s
albinsuresh commented 1 month ago

However, the code related to health status is still fragile because unstructured.

  • There are two similar HealthStatus structs defined by c8y related crates (c8y_api and c8y_mapper_ext).
  • The respective roles of tedge_api and c8y_api is not clear. Several methods should be moved into tedge_api to encapsulate all the tedge conventions.
  • This PR introduces UP_STATUS and DOWN_STATUS constants, but "up" and "down" literals are used everywhere.

I completely agree and this was the cleanup that I wanted to do as well. But, kept the minimal changes in this PR because of the urgency in fixing the bug. Will definitely do a follow-up PR with all that cleanup.