refactor: add support to define_tedge_config for (optionally) multi-value fields#3126
Merged
jarhodes314 merged 32 commits intothin-edge:mainfrom Oct 7, 2024
Merged
Conversation
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
jarhodes314
commented
Sep 20, 2024
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Contributor
Robot Results
|
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Co-authored-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
c683980 to
64ddb22
Compare
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
jarhodes314
commented
Oct 2, 2024
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
Signed-off-by: James Rhodes <jarhodes314@gmail.com>
d32d6b1 to
3936b7d
Compare
didier-wenzek
approved these changes
Oct 7, 2024
Contributor
didier-wenzek
left a comment
There was a problem hiding this comment.
Approved. Thank you for this tour de force.
Despite its underneath complexity, tedge config is simple to use both by end users and developers (despite or thanks to this complexity?). However, we reached a point where things are really complicated and I would be very careful about adding additional features.
| new_parents.push(PathItem::Dynamic(*span)); | ||
| } | ||
| } else { | ||
| // This key has diverged from the currrent key's parents, so empty the list |
Contributor
There was a problem hiding this comment.
Suggested change
| // This key has diverged from the currrent key's parents, so empty the list | |
| // This key has diverged from the current key's parents, so empty the list |
Comment on lines
+47
to
+49
| #[error("Unexpected profile {1} for the non multi-profile property {0}")] | ||
| #[error( | ||
| "You are trying to access a profile `{1}` of {0}, but profiles are not enabled for {0}" | ||
| )] |
Contributor
There was a problem hiding this comment.
I have a preference for the former error message.
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
In order to add support multiple Cumulocity connections, we need the data model defined for
tedge.tomlto support multiple cloud connections. This PR adds support indefine_tedge_config!for a#[tedge_config(multi)]attribute, which allows the field to store either the underlying struct directly or a hashmap of strings to structs (which provides us with multiple named configurations for a single key).More concretely, this will allow
tedge_configto support configurations like:while still also supporting as it does currently:
This PR doesn't make any changes to the existing
tedge_configdefinitions and therefore doesn't change the behaviour of thin-edge, hence the refactoring classification.Still TODO:
tedge config listlogicmatchblocks to_Types of changes
Paste Link to the issue
Checklist
cargo fmtas mentioned in CODING_GUIDELINEScargo clippyas mentioned in CODING_GUIDELINESFurther comments