Add #[derive(Document)] to TEdgeConfigDto and replace FilePath with camino::Utf8PathBuf #1867

jarhodes314 · 2023-03-29T15:36:39Z

Proposed changes

This is the first batch of changes for #1812. It adds documentation to the TEdgeConfigDto struct via doku. It also replaces FilePath with camino::Utf8Path for a few reasons:

The internals of TEdgeConfigDto will become more visible as part of later changes
I don't think we can ever end up processing non UTF-8 paths as config files, environment variables, JSON from Cumulocity etc. all require valid unicode as input
The code frequently assumes paths are UTF-8 anyway, either calling .to_str() or storing them as Strings when we need to display them (e.g. writing to mosquitto configuration)

From that, I believe camino is a good fit for our use case. I've not propagated the changes beyond a few crates, but I think I've covered the major interactions with tedge_config.

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

#1812

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)

didier-wenzek

Some minor comments.

The output is not consistent regarding the separator between doc and example: some entries are missing a dot, others a space, some both:

$ tedge config list --doc                       
device.type                    The default device type. Example: thin-edge.io
http.address                   Http client address, which is used by the File Transfer ServiceExample: 127.0.0.1, 192.168.1.2
mqtt.client.host               Mqtt broker address, which is used by the mqtt clients to publish or subscribe.Example: 127.0.0.1
...

crates/common/tedge_config/src/tedge_config_cli/models/connect_url.rs

crates/common/tedge_config/Cargo.toml

didier-wenzek · 2023-03-29T16:38:34Z

crates/common/tedge_config/src/tedge_config_cli/tedge_config_dto.rs

+    // TODO improve these examples
+    #[doku(example = "template1")]
+    #[doku(example = "template2")]
+    /// Set of c8y templates used for subscriptions
    pub(crate) smartrest_templates: Option<TemplatesSet>,


Rather than the examples, I think what needs to be improved is the field documentation. These are not templates but template names and they must be defined on c8y.

Also, from the examples, it's not clear how one can set a list of template names. As far I understand doku these two examples are independent as those given for the bind_address. I would expect a sample that is actually a set.

Yeah, the doku example is a bit weird, but this does translate to a list when generating the documentation.

…amino::Utf8PathBuf Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

jarhodes314 · 2023-03-29T17:09:24Z

Some minor comments.

The output is not consistent regarding the separator between doc and example: some entries are missing a dot, others a space, some both:

$ tedge config list --doc                       
device.type                    The default device type. Example: thin-edge.io
http.address                   Http client address, which is used by the File Transfer ServiceExample: 127.0.0.1, 192.168.1.2
mqtt.client.host               Mqtt broker address, which is used by the mqtt clients to publish or subscribe.Example: 127.0.0.1
...

At the moment, that is still using the existing implementation, although I plan to start working on changing that next.

didier-wenzek

Approved

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

github-actions · 2023-03-29T18:18:02Z

Robot Results

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

Passed Tests

Name	⏱️ Duration	Suite
Define Child device 1 ID	0.098 s	`C8Y Child Alarms Rpi`
Normal case when the child device does not exist on c8y cloud	3.585 s	`C8Y Child Alarms Rpi`
Normal case when the child device already exists	1.222 s	`C8Y Child Alarms Rpi`
Reconciliation when the new alarm message arrives, restart the mapper	2.137 s	`C8Y Child Alarms Rpi`
Reconciliation when the alarm that is cleared	6.463 s	`C8Y Child Alarms Rpi`
Prerequisite Parent	23.152 s	`Child Conf Mgmt Plugin`
Prerequisite Child	0.911 s	`Child Conf Mgmt Plugin`
Child device bootstrapping	20.259 s	`Child Conf Mgmt Plugin`
Snapshot from device	61.74 s	`Child Conf Mgmt Plugin`
Child device config update	63.552 s	`Child Conf Mgmt Plugin`
Configuration types should be detected on file change (without restarting service)	68.409 s	`Inotify Crate`
Child devices support sending simple measurements	2.4779999999999998 s	`Child Device Telemetry`
Child devices support sending custom measurements	1.297 s	`Child Device Telemetry`
Child devices support sending custom events	1.207 s	`Child Device Telemetry`
Child devices support sending custom events overriding the type	1.7029999999999998 s	`Child Device Telemetry`
Child devices support sending custom alarms #1699	2.088 s	`Child Device Telemetry`
Child devices support sending inventory data via c8y topic	1.721 s	`Child Device Telemetry`
Main device support sending inventory data via c8y topic	2.277 s	`Child Device Telemetry`
Child device supports sending custom child device measurements directly to c8y	2.349 s	`Child Device Telemetry`
Main device supports sending custom child device measurements directly to c8y	2.123 s	`Child Device Telemetry`
Successful firmware operation	88.043 s	`Firmware Operation`
Install with empty firmware name	74.81 s	`Firmware Operation`
Prerequisite Parent	27.486 s	`Firmware Operation Child Device`
Prerequisite Child	9.079 s	`Firmware Operation Child Device`
Child device firmware update	4.851 s	`Firmware Operation Child Device`
Child device firmware update with cache	7.083 s	`Firmware Operation Child Device`
Update Inventory data via inventory.json	1.97 s	`Inventory Update`
Retrieve a JWT tokens	73.242 s	`Jwt Request`
Main device registration	2.784 s	`Device Registration`
Child device registration	3.368 s	`Device Registration`
Supports restarting the device	96.131 s	`Restart Device`
Update tedge version from previous using Cumulocity	129.96 s	`Tedge Self Update`
Test if all c8y services are up	87.928 s	`Service Monitoring`
Test if all c8y services are down	77.538 s	`Service Monitoring`
Test if all c8y services are using configured service type	79.751 s	`Service Monitoring`
Test if all c8y services using default service type when service type configured as empty	44.97 s	`Service Monitoring`
Check health status of tedge-mapper-c8y service on broker stop start	35.424 s	`Service Monitoring`
Check health status of tedge-mapper-c8y service on broker restart	38.951 s	`Service Monitoring`
Check health status of child device service	28.102 s	`Service Monitoring`
Successful shell command with output	4.2379999999999995 s	`Shell Operation`
Check Successful shell command with literal double quotes output	3.8689999999999998 s	`Shell Operation`
Execute multiline shell command	3.597 s	`Shell Operation`
Failed shell command	3.573 s	`Shell Operation`
Software list should be populated during startup	68.995 s	`Software`
Install software via Cumulocity	86.66 s	`Software`
Software list should only show currently installed software and not candidates	65.993 s	`Software`
Validate updated data path used by tedge-agent	1.028 s	`Data Path Config`
Validate updated data path used by c8y-firmware-plugin	11.619 s	`Data Path Config`
Stop tedge-agent service	0.764 s	`Log Path Config`
Customize the log path	0.267 s	`Log Path Config`
Initialize tedge-agent	0.35 s	`Log Path Config`
Check created folders	0.185 s	`Log Path Config`
Remove created custom folders	0.206 s	`Log Path Config`
Install thin-edge via apt	30.208 s	`Install Apt`
Install latest via script (from current branch)	40.787 s	`Install Tedge`
Install specific version via script (from current branch)	18.715 s	`Install Tedge`
Install latest tedge via script (from main branch)	30.471 s	`Install Tedge`
Support starting and stopping services	60.438 s	`Service-Control`
Supports a reconnect	72.633 s	`Test-Commands`
Supports disconnect then connect	66.41 s	`Test-Commands`
Update unknown setting	40.166 s	`Test-Commands`
Update known setting	27.783 s	`Test-Commands`
Stop c8y-configuration-plugin	0.259 s	`Health C8Y-Configuration-Plugin`
Update the service file	0.363 s	`Health C8Y-Configuration-Plugin`
Reload systemd files	1.592 s	`Health C8Y-Configuration-Plugin`
Start c8y-configuration-plugin	0.326 s	`Health C8Y-Configuration-Plugin`
Start watchdog service	10.497 s	`Health C8Y-Configuration-Plugin`
Check PID of c8y-configuration-plugin	0.11 s	`Health C8Y-Configuration-Plugin`
Kill the PID	0.261 s	`Health C8Y-Configuration-Plugin`
Recheck PID of c8y-configuration-plugin	0.15 s	`Health C8Y-Configuration-Plugin`
Compare PID change	0.001 s	`Health C8Y-Configuration-Plugin`
Stop watchdog service	0.222 s	`Health C8Y-Configuration-Plugin`
Remove entry from service file	0.151 s	`Health C8Y-Configuration-Plugin`
Stop c8y-log-plugin	0.405 s	`Health C8Y-Log-Plugin`
Update the service file	0.382 s	`Health C8Y-Log-Plugin`
Reload systemd files	1.153 s	`Health C8Y-Log-Plugin`
Start c8y-log-plugin	0.234 s	`Health C8Y-Log-Plugin`
Start watchdog service	10.205 s	`Health C8Y-Log-Plugin`
Check PID of c8y-log-plugin	0.099 s	`Health C8Y-Log-Plugin`
Kill the PID	0.201 s	`Health C8Y-Log-Plugin`
Recheck PID of c8y-log-plugin	2.412 s	`Health C8Y-Log-Plugin`
Compare PID change	0.003 s	`Health C8Y-Log-Plugin`
Stop watchdog service	0.367 s	`Health C8Y-Log-Plugin`
Remove entry from service file	0.471 s	`Health C8Y-Log-Plugin`
Stop tedge-mapper	0.412 s	`Health Tedge Mapper C8Y`
Update the service file	0.313 s	`Health Tedge Mapper C8Y`
Reload systemd files	0.922 s	`Health Tedge Mapper C8Y`
Start tedge-mapper	0.227 s	`Health Tedge Mapper C8Y`
Start watchdog service	10.34 s	`Health Tedge Mapper C8Y`
Check PID of tedge-mapper	0.207 s	`Health Tedge Mapper C8Y`
Kill the PID	0.519 s	`Health Tedge Mapper C8Y`
Recheck PID of tedge-mapper	0.271 s	`Health Tedge Mapper C8Y`
Compare PID change	0.002 s	`Health Tedge Mapper C8Y`
Stop watchdog service	0.162 s	`Health Tedge Mapper C8Y`
Remove entry from service file	0.178 s	`Health Tedge Mapper C8Y`
Stop tedge-agent	0.306 s	`Health Tedge-Agent`
Update the service file	0.184 s	`Health Tedge-Agent`
Reload systemd files	0.871 s	`Health Tedge-Agent`
Start tedge-agent	0.213 s	`Health Tedge-Agent`
Start watchdog service	10.502 s	`Health Tedge-Agent`
Check PID of tedge-mapper	0.109 s	`Health Tedge-Agent`
Kill the PID	0.192 s	`Health Tedge-Agent`
Recheck PID of tedge-agent	2.438 s	`Health Tedge-Agent`
Compare PID change	0.002 s	`Health Tedge-Agent`
Stop watchdog service	0.374 s	`Health Tedge-Agent`
Remove entry from service file	0.256 s	`Health Tedge-Agent`
Stop tedge-mapper-az	0.407 s	`Health Tedge-Mapper-Az`
Update the service file	0.235 s	`Health Tedge-Mapper-Az`
Reload systemd files	1.042 s	`Health Tedge-Mapper-Az`
Start tedge-mapper-az	0.455 s	`Health Tedge-Mapper-Az`
Start watchdog service	10.408 s	`Health Tedge-Mapper-Az`
Check PID of tedge-mapper-az	0.137 s	`Health Tedge-Mapper-Az`
Kill the PID	0.379 s	`Health Tedge-Mapper-Az`
Recheck PID of tedge-agent	2.518 s	`Health Tedge-Mapper-Az`
Compare PID change	0.005 s	`Health Tedge-Mapper-Az`
Stop watchdog service	0.215 s	`Health Tedge-Mapper-Az`
Remove entry from service file	0.137 s	`Health Tedge-Mapper-Az`
Stop tedge-mapper-collectd	0.182 s	`Health Tedge-Mapper-Collectd`
Update the service file	0.124 s	`Health Tedge-Mapper-Collectd`
Reload systemd files	0.653 s	`Health Tedge-Mapper-Collectd`
Start tedge-mapper-collectd	0.14 s	`Health Tedge-Mapper-Collectd`
Start watchdog service	10.226 s	`Health Tedge-Mapper-Collectd`
Check PID of tedge-mapper-collectd	0.226 s	`Health Tedge-Mapper-Collectd`
Kill the PID	0.315 s	`Health Tedge-Mapper-Collectd`
Recheck PID of tedge-mapper-collectd	0.181 s	`Health Tedge-Mapper-Collectd`
Compare PID change	0.002 s	`Health Tedge-Mapper-Collectd`
Stop watchdog service	0.229 s	`Health Tedge-Mapper-Collectd`
Remove entry from service file	0.196 s	`Health Tedge-Mapper-Collectd`
c8y-log-plugin health status	6.136 s	`MQTT health endpoints`
c8y-configuration-plugin health status	6.266 s	`MQTT health endpoints`
Wrong package name	0.269 s	`Improve Tedge Apt Plugin Error Messages`
Wrong version	0.321 s	`Improve Tedge Apt Plugin Error Messages`
Wrong type	0.737 s	`Improve Tedge Apt Plugin Error Messages`
tedge_connect_test_positive	0.861 s	`Tedge Connect Test`
tedge_connect_test_negative	2.148 s	`Tedge Connect Test`
tedge_connect_test_sm_services	10.693999999999999 s	`Tedge Connect Test`
tedge_disconnect_test_sm_services	1.6139999999999999 s	`Tedge Connect Test`
Install thin-edge.io	26.877 s	`Call Tedge`
call tedge -V	0.143 s	`Call Tedge`
call tedge -h	0.104 s	`Call Tedge`
call tedge -h -V	0.129 s	`Call Tedge`
call tedge help	0.098 s	`Call Tedge`
tedge config list	0.184 s	`Call Tedge Config List`
tedge config list --all	0.2 s	`Call Tedge Config List`
set/unset device.type	1.177 s	`Call Tedge Config List`
set/unset device.key.path	0.74 s	`Call Tedge Config List`
set/unset device.cert.path	1.308 s	`Call Tedge Config List`
set/unset c8y.root.cert.path	1.199 s	`Call Tedge Config List`
set/unset c8y.smartrest.templates	0.791 s	`Call Tedge Config List`
set/unset az.root.cert.path	0.738 s	`Call Tedge Config List`
set/unset az.mapper.timestamp	0.642 s	`Call Tedge Config List`
set/unset mqtt.bind_address	0.511 s	`Call Tedge Config List`
set/unset mqtt.port	0.539 s	`Call Tedge Config List`
set/unset tmp.path	0.847 s	`Call Tedge Config List`
set/unset logs.path	0.902 s	`Call Tedge Config List`
set/unset run.path	0.926 s	`Call Tedge Config List`
Get Put Delete	3.858 s	`Http File Transfer Api`
Set keys should return value on stdout	0.086 s	`Tedge Config Get`
Unset keys should not return anything on stdout and warnings on stderr	0.153 s	`Tedge Config Get`
Invalid keys should not return anything on stdout and warnings on stderr	0.136 s	`Tedge Config Get`
Set configuration via environment variables	0.481 s	`Tedge Config Get`
Set unknown configuration via environment variables	0.067 s	`Tedge Config Get`

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

jarhodes314 requested review from didier-wenzek and Bravo555 March 29, 2023 15:36

jarhodes314 force-pushed the feat/doku-camino branch from 7184ced to b5b824f Compare March 29, 2023 15:47

didier-wenzek reviewed Mar 29, 2023

View reviewed changes

jarhodes314 added 2 commits March 29, 2023 18:03

Add #[derive(Document)] to TEdgeConfigDto and replace FilePath with c…

d6b99ff

…amino::Utf8PathBuf Signed-off-by: James Rhodes <jarhodes314@gmail.com>

Remove test that was purely for inspecting the output

d9db8dc

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

jarhodes314 force-pushed the feat/doku-camino branch from b5b824f to 954e933 Compare March 29, 2023 17:07

didier-wenzek approved these changes Mar 29, 2023

View reviewed changes

jarhodes314 force-pushed the feat/doku-camino branch from 954e933 to cb0cc99 Compare March 29, 2023 17:10

jarhodes314 had a problem deploying to Test Pull Request March 29, 2023 17:22 — with GitHub Actions Failure

Use doku(as) to remove manual implementation

1814453

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

jarhodes314 force-pushed the feat/doku-camino branch from 650ff44 to 1814453 Compare March 29, 2023 17:45

jarhodes314 temporarily deployed to Test Pull Request March 29, 2023 17:52 — with GitHub Actions Inactive

Run cargo sort

8e53366

Signed-off-by: James Rhodes <jarhodes314@gmail.com>

jarhodes314 had a problem deploying to Test Pull Request March 29, 2023 18:34 — with GitHub Actions Failure

jarhodes314 temporarily deployed to Test Pull Request March 29, 2023 20:43 — with GitHub Actions Inactive

jarhodes314 merged commit c726620 into thin-edge:main Mar 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add #[derive(Document)] to TEdgeConfigDto and replace FilePath with camino::Utf8PathBuf #1867

Add #[derive(Document)] to TEdgeConfigDto and replace FilePath with camino::Utf8PathBuf #1867

jarhodes314 commented Mar 29, 2023 •

edited

didier-wenzek left a comment

didier-wenzek Mar 29, 2023

jarhodes314 Mar 29, 2023

jarhodes314 commented Mar 29, 2023

didier-wenzek left a comment

github-actions bot commented Mar 29, 2023 •

edited

Add #[derive(Document)] to TEdgeConfigDto and replace FilePath with camino::Utf8PathBuf #1867

Add #[derive(Document)] to TEdgeConfigDto and replace FilePath with camino::Utf8PathBuf #1867

Conversation

jarhodes314 commented Mar 29, 2023 • edited

Proposed changes

Types of changes

Paste Link to the issue

Checklist

didier-wenzek left a comment

Choose a reason for hiding this comment

didier-wenzek Mar 29, 2023

Choose a reason for hiding this comment

jarhodes314 Mar 29, 2023

Choose a reason for hiding this comment

jarhodes314 commented Mar 29, 2023

didier-wenzek left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 29, 2023 • edited

Robot Results

Passed Tests

jarhodes314 commented Mar 29, 2023 •

edited

github-actions bot commented Mar 29, 2023 •

edited