YAML model repository: supports only version 1 with elements as map #4807
Conversation
|
This PR has to be merged after #4762 |
To be merged once the upgrade tool is available Signed-off-by: Laurent Garnier <lg.hc@free.fr>
lolodomo
force-pushed
the
yaml_config_remove_list
branch
from
03782a4 to
2a01e34
Compare
|
What would happen if a yaml file is found with tags in array syntax (e.g. the upgrade tool failed or didn't run)? |
|
Tags will be ignored. |
|
Would you like to add a specific warning log? |
|
A warning might be good |
Signed-off-by: Laurent Garnier <lg.hc@free.fr>
|
@lolodomo May I ask you to rebase this in order to resolve the merge conflict? |
Done |
Required after the merge of PR openhab#4807 and PR openhab#4776 Signed-off-by: Laurent Garnier <lg.hc@free.fr>
| logger.trace("Element {} in model {} is not a container object, ignoring it", elementName, | ||
| if (!node.isContainerNode() || node.isArray()) { | ||
| // all processable sub-elements are container nodes (not array) | ||
| logger.trace("Element {} in model {} is not a container object, ignoring it", elementName, |
There was a problem hiding this comment.
@lolodomo I think this should be a warn as well, I just tested my "old" V1 rules, and nothing was logged until I turned on trace. I know that the file is no longer valid, but using an array instead of a map is an "easy mistake to make" for users, and as it is logged it seems like everything is OK, but the element is ignored and doesn't show up.
There was a problem hiding this comment.
Probably not wished generally but could be a good idea in case of a node we are knowing. So could be useful for node "things" but not for node "dummy".
There was a problem hiding this comment.
I don't know what "the general wishes" regarding logging are, but I think it's important that users get feedback when they make a mistake so that they can correct it. All that is logged now is "Adding YAML model " (INFO) and then nothing that indicates that something went wrong. But the object never shows up in the system.
Edit: As I understand you, to do that, you'd have to maintain a list of "known node names" to check against? What if the user misspells the node name?
There was a problem hiding this comment.
I am not against a general warning finally.
To be merged once the upgrade tool is available