Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove tedge_actors dependency on anyhow #1752

Merged
merged 2 commits into from
Feb 27, 2023
Merged

Remove tedge_actors dependency on anyhow #1752

merged 2 commits into from
Feb 27, 2023

Conversation

didier-wenzek
Copy link
Contributor

A library crate should not depend on anyhow

Proposed changes

  • Remove tedge_actors dependency on anyhow
  • Remove c8y_config_manager dependency on anyhow
  • Extends RuntimeError with a case for any std::error::Error
  • Change the signature of Actor::run to raise RuntimeError

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#1724

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I ran cargo fmt as mentioned in CODING_GUIDELINES
  • I used cargo clippy as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@didier-wenzek didier-wenzek temporarily deployed to Test Pull Request February 23, 2023 16:01 — with GitHub Actions Inactive
albinsuresh
albinsuresh reviewed Feb 24, 2023
View reviewed changes
crates/extensions/c8y_http_proxy/src/actor.rs Outdated
let actor = C8YHttpProxyActor::new(self, messages);
actor.run().await
Ok(actor.run().await?)
Copy link
Contributor

Choose a reason for hiding this comment

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

That looks weird. Wondering why the C8YHttpProxyActor::run is not returning RuntimeError

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. This has been fixed by f9fd119

crates/extensions/c8y_config_manager/src/actor.rs Outdated
@@ -64,7 +65,7 @@ impl ConfigManagerActor {
&mut self,
message: MqttMessage,
message_box: &mut ConfigManagerMessageBox,
) -> Result<(), anyhow::Error> {
) -> Result<(), DynError> {
Copy link
Contributor

Choose a reason for hiding this comment

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

That's my bad. I'll do a follow-up PR to fix this by makes all config manager functions return ConfigManagementError instead of this DynError.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would even be good to do this in that PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done as a fixup commit f9fd119

@github-actions
Copy link
Contributor

github-actions bot commented Feb 24, 2023 •
edited

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
137 0 5 137 100

Passed Tests

Name ⏱️ Duration Suite
Define Child device 1 ID 0.024 s C8Y Child Alarms Rpi
Normal case when the child device does not exist on c8y cloud 1.897 s C8Y Child Alarms Rpi
Normal case when the child device already exists 0.731 s C8Y Child Alarms Rpi
Reconciliation when the new alarm message arrives, restart the mapper 1.683 s C8Y Child Alarms Rpi
Reconciliation when the alarm that is cleared 5.549 s C8Y Child Alarms Rpi
Prerequisite Parent 18.654 s Child Conf Mgmt Plugin
Prerequisite Child 0.282 s Child Conf Mgmt Plugin
Child device bootstrapping 15.412 s Child Conf Mgmt Plugin
Snapshot from device 19.247 s Child Conf Mgmt Plugin
Child device config update 14.72 s Child Conf Mgmt Plugin
Configuration types should be detected on file change (without restarting service) 45.971 s Inotify Crate
Child devices support sending simple measurements 46.368 s Child Device Telemetry
Child devices support sending custom measurements 44.739 s Child Device Telemetry
Child devices support sending custom events 40.476 s Child Device Telemetry
Child devices support sending custom events overriding the type 36.891 s Child Device Telemetry
Child devices support sending custom alarms #1699 33.591 s Child Device Telemetry
Child devices support sending inventory data via c8y topic 23.646 s Child Device Telemetry
Main device support sending inventory data via c8y topic 24.654 s Child Device Telemetry
Successful firmware operation 59.843 s Firmware Operation
Install with empty firmware name 53.921 s Firmware Operation
Supports restarting the device 74.937 s Restart Device
Update tedge version from previous using Cumulocity 106.97 s Tedge Self Update
Successful shell command with output 3.652 s Shell Operation
Check Successful shell command with literal double quotes output 3.335 s Shell Operation
Execute multiline shell command 2.992 s Shell Operation
Failed shell command 3.187 s Shell Operation
Software list should be populated during startup 52.306 s Software
Install software via Cumulocity 72.072 s Software
Software list should only show currently installed software and not candidates 43.758 s Software
Stop tedge-agent service 0.328 s Log Path Config
Customize the log path 0.103 s Log Path Config
Initialize tedge-agent 0.213 s Log Path Config
Check created folders 0.086 s Log Path Config
Remove created custom folders 0.15 s Log Path Config
Install latest via script (from current branch) 29.85 s Install Tedge
Install specific version via script (from current branch) 18.311 s Install Tedge
Install latest tedge via script (from main branch) 23.445 s Install Tedge
Support starting and stopping services 38.584 s Service-Control
Supports a reconnect 49.588 s Test-Commands
Supports disconnect then connect 48.649 s Test-Commands
Update unknown setting 29.489 s Test-Commands
Update known setting 26.045 s Test-Commands
Stop c8y-configuration-plugin 0.23 s Health C8Y-Configuration-Plugin
Update the service file 0.183 s Health C8Y-Configuration-Plugin
Reload systemd files 0.775 s Health C8Y-Configuration-Plugin
Start c8y-configuration-plugin 0.221 s Health C8Y-Configuration-Plugin
Start watchdog service 10.352 s Health C8Y-Configuration-Plugin
Check PID of c8y-configuration-plugin 0.208 s Health C8Y-Configuration-Plugin
Kill the PID 0.334 s Health C8Y-Configuration-Plugin
Recheck PID of c8y-configuration-plugin 0.197 s Health C8Y-Configuration-Plugin
Compare PID change 0.008 s Health C8Y-Configuration-Plugin
Stop watchdog service 0.236 s Health C8Y-Configuration-Plugin
Remove entry from service file 0.232 s Health C8Y-Configuration-Plugin
Stop c8y-log-plugin 0.333 s Health C8Y-Log-Plugin
Update the service file 0.28 s Health C8Y-Log-Plugin
Reload systemd files 1.255 s Health C8Y-Log-Plugin
Start c8y-log-plugin 0.208 s Health C8Y-Log-Plugin
Start watchdog service 10.473 s Health C8Y-Log-Plugin
Check PID of c8y-log-plugin 0.095 s Health C8Y-Log-Plugin
Kill the PID 0.105 s Health C8Y-Log-Plugin
Recheck PID of c8y-log-plugin 2.141 s Health C8Y-Log-Plugin
Compare PID change 0.001 s Health C8Y-Log-Plugin
Stop watchdog service 0.084 s Health C8Y-Log-Plugin
Remove entry from service file 0.079 s Health C8Y-Log-Plugin
Stop tedge-mapper 0.253 s Health Tedge Mapper C8Y
Update the service file 0.254 s Health Tedge Mapper C8Y
Reload systemd files 1.089 s Health Tedge Mapper C8Y
Start tedge-mapper 0.198 s Health Tedge Mapper C8Y
Start watchdog service 10.335 s Health Tedge Mapper C8Y
Check PID of tedge-mapper 0.088 s Health Tedge Mapper C8Y
Kill the PID 0.091 s Health Tedge Mapper C8Y
Recheck PID of tedge-mapper 2.138 s Health Tedge Mapper C8Y
Compare PID change 0.001 s Health Tedge Mapper C8Y
Stop watchdog service 0.132 s Health Tedge Mapper C8Y
Remove entry from service file 0.086 s Health Tedge Mapper C8Y
Stop tedge-agent 0.152 s Health Tedge-Agent
Update the service file 0.084 s Health Tedge-Agent
Reload systemd files 0.312 s Health Tedge-Agent
Start tedge-agent 0.079 s Health Tedge-Agent
Start watchdog service 10.186 s Health Tedge-Agent
Check PID of tedge-mapper 0.099 s Health Tedge-Agent
Kill the PID 0.105 s Health Tedge-Agent
Recheck PID of tedge-agent 2.39 s Health Tedge-Agent
Compare PID change 0.001 s Health Tedge-Agent
Stop watchdog service 0.192 s Health Tedge-Agent
Remove entry from service file 0.179 s Health Tedge-Agent
Stop tedge-mapper-az 0.205 s Health Tedge-Mapper-Az
Update the service file 0.158 s Health Tedge-Mapper-Az
Reload systemd files 0.703 s Health Tedge-Mapper-Az
Start tedge-mapper-az 0.333 s Health Tedge-Mapper-Az
Start watchdog service 10.219 s Health Tedge-Mapper-Az
Check PID of tedge-mapper-az 0.116 s Health Tedge-Mapper-Az
Kill the PID 0.183 s Health Tedge-Mapper-Az
Recheck PID of tedge-agent 2.279 s Health Tedge-Mapper-Az
Compare PID change 0.001 s Health Tedge-Mapper-Az
Stop watchdog service 0.119 s Health Tedge-Mapper-Az
Remove entry from service file 0.115 s Health Tedge-Mapper-Az
Stop tedge-mapper-collectd 0.206 s Health Tedge-Mapper-Collectd
Update the service file 0.184 s Health Tedge-Mapper-Collectd
Reload systemd files 0.829 s Health Tedge-Mapper-Collectd
Start tedge-mapper-collectd 0.187 s Health Tedge-Mapper-Collectd
Start watchdog service 10.275 s Health Tedge-Mapper-Collectd
Check PID of tedge-mapper-collectd 0.231 s Health Tedge-Mapper-Collectd
Kill the PID 0.187 s Health Tedge-Mapper-Collectd
Recheck PID of tedge-mapper-collectd 2.289 s Health Tedge-Mapper-Collectd
Compare PID change 0.017 s Health Tedge-Mapper-Collectd
Stop watchdog service 0.26 s Health Tedge-Mapper-Collectd
Remove entry from service file 0.269 s Health Tedge-Mapper-Collectd
c8y-log-plugin health status 5.7059999999999995 s MQTT health endpoints
c8y-configuration-plugin health status 5.627 s MQTT health endpoints
Wrong package name 0.173 s Improve Tedge Apt Plugin Error Messages
Wrong version 0.15 s Improve Tedge Apt Plugin Error Messages
Wrong type 0.377 s Improve Tedge Apt Plugin Error Messages
tedge_connect_test_positive 0.555 s Tedge Connect Test
tedge_connect_test_negative 0.934 s Tedge Connect Test
tedge_connect_test_sm_services 6.674 s Tedge Connect Test
tedge_disconnect_test_sm_services 0.315 s Tedge Connect Test
Install thin-edge.io 12.506 s Call Tedge
call tedge -V 0.226 s Call Tedge
call tedge -h 0.159 s Call Tedge
call tedge -h -V 0.151 s Call Tedge
call tedge help 0.138 s Call Tedge
tedge config list 0.118 s Call Tedge Config List
tedge config list --all 0.103 s Call Tedge Config List
set/unset device.type 0.645 s Call Tedge Config List
set/unset device.key.path 0.86 s Call Tedge Config List
set/unset device.cert.path 0.923 s Call Tedge Config List
set/unset c8y.root.cert.path 0.8 s Call Tedge Config List
set/unset c8y.smartrest.templates 0.488 s Call Tedge Config List
set/unset az.root.cert.path 0.379 s Call Tedge Config List
set/unset az.mapper.timestamp 0.39 s Call Tedge Config List
set/unset mqtt.bind_address 0.31 s Call Tedge Config List
set/unset mqtt.port 0.529 s Call Tedge Config List
set/unset tmp.path 0.431 s Call Tedge Config List
set/unset logs.path 0.462 s Call Tedge Config List
set/unset run.path 0.432 s Call Tedge Config List
Get Put Delete 4.173 s Http File Transfer Api

@didier-wenzek didier-wenzek temporarily deployed to Test Pull Request February 24, 2023 13:58 — with GitHub Actions Inactive
@didier-wenzek didier-wenzek temporarily deployed to Test Pull Request February 27, 2023 08:23 — with GitHub Actions Inactive
@didier-wenzek didier-wenzek temporarily deployed to Test Pull Request February 27, 2023 14:26 — with GitHub Actions Inactive
@didier-wenzek
Remove tedge_actors dependency on anyhow
787323c 
- A library crate should not depend on anyhow
- Extends RuntimeError with a case for any std::error::Error
- Change the signature of Actor::run to raise RuntimeError

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek didier-wenzek temporarily deployed to Test Pull Request February 27, 2023 15:22 — with GitHub Actions Inactive
@didier-wenzek
Fix error on test report generation
5d881ea 
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek didier-wenzek temporarily deployed to Test Pull Request February 27, 2023 17:34 — with GitHub Actions Inactive
@didier-wenzek
Copy link
Contributor Author

Merging this PR despite test report has not been generated: the fix can be effective only on the main branch.

@didier-wenzek didier-wenzek merged commit 1777f01 into thin-edge:main Feb 27, 2023
@didier-wenzek didier-wenzek deleted the refactor/remove-dep-on-anyhow branch February 27, 2023 18:05
@didier-wenzek didier-wenzek mentioned this pull request Mar 14, 2023
Closed
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albinsuresh albinsuresh albinsuresh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@didier-wenzek @albinsuresh