feat: create parent directories if missing for config_update operation#3718
Conversation
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
Robot Results
|
tests/RobotFramework/tests/cumulocity/configuration/tedge-configuration-plugin.toml
Outdated
Show resolved
Hide resolved
didier-wenzek
left a comment
There was a problem hiding this comment.
Approved.
This PR has to be rebased on top of #3727, to have the broken unit tests fixed.
1596df4 to
9876dda
Compare
9876dda to
59af83e
Compare
tests/RobotFramework/tests/cumulocity/configuration/tedge-configuration-plugin.toml
Outdated
Show resolved
Hide resolved
tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot
Show resolved
Hide resolved
| ${hash_before}= Execute Command md5sum ${device_file} ||true | ||
| ${stat_before}= Execute Command stat ${device_file} ||true |
There was a problem hiding this comment.
That || true was intentional? If the expected file doesn't exist upfront (when it is expected to exist), wouldn't it be better to have the tests fail here with the clear "file not found error" rather than in the file content equiality check failing later on(which could be misleading)?
There was a problem hiding this comment.
@reubenmiller Can I remove || true as Albin suggested?
There was a problem hiding this comment.
Though the downside is that the Execute Command will take 30 seconds to fail if the file isn't found (due to the in-built "smart retry" mechanism.
If you want clarify I would suggest using the "File Exists" check prior to getting it, as that will be the most descriptive.
There was a problem hiding this comment.
Though given that the ||true was only added afterwards, then it might be better to remove it as it seems to be just a by-product of the PR and not directly required by it.
There was a problem hiding this comment.
Added ThinEdgeIo.File Should Exist and remove ||true. 6a61165
6a61165 to
94b0a27
Compare
Previously, the `config_update` operation would fail if any parent directories of the target file were missing. This change ensures that missing parent directories are automatically created when the operation is executed. The mode, user, and group of the immediate parent directory can be configured via `tedge-configuration-plugin.toml` using the following keys: - `parent_mode` - `parent_user` - `parent_group` If `parent_user` and `parent_group` are not explicitly specified, but the target file's ownership is provided, the parent directory will inherit the file's user and group. Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>
Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
…settings Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>
94b0a27 to
59d5806
Compare
Proposed changes
Previously, the
config_updateoperation would fail if any parent directories of the target file were missing.This change ensures that missing parent directories are automatically created when the operation is executed.
The mode, user, and group of the immediate parent directory can be configured via
tedge-configuration-plugin.tomlusing the following keys:parent_modeparent_userparent_groupIf
parent_userandparent_groupare not explicitly specified, but the target file's ownership is provided, the parent directory will inherit the file's user and group.Types of changes
Paste Link to the issue
#3693
Checklist
just prepare-devonce)just formatas mentioned in CODING_GUIDELINESjust checkas mentioned in CODING_GUIDELINESFurther comments