Allow some Sonobuoy tests (KaaS) to fail.#1092
Conversation
0cda8ea to
7d265d6
Compare
| if not isinstance(config_obj, dict): | ||
| raise ValueError(f"Invalid sonobuoy config format in {sonobuoy_config}: top-level YAML object must be a mapping") |
There was a problem hiding this comment.
I would omit all the syntax checks for the YAML. If we want that, we can use some kind of YAML schema validator or something. I would do minimal checks, but I would catch any exception that occurs while loading the file saying there might be a syntax error. We don't expect that to happen often because we control the YAML file ourselves, and we don't expect it to change very often. Most likely change is that we remove entries once the issues with Cilium go away one by one.
There was a problem hiding this comment.
@mbuechse I would like the code to fail, if the config contains typos. I would like the command to fail, if someone writes "regexs" instead of "regex". But if it is very important for you, I will remove the syntax checks.
There was a problem hiding this comment.
I suppose the code will fail with a KeyError or NameError, which is not as nice. I think using a schema validator (if we deem it necessary) would spare us from a lot of boilerplate code here. But I won't die on a hill fighting over this...
There was a problem hiding this comment.
Then I would like to leave the code as it is. I really do not like it, when a typo in a config get silently ignored.
f8a482c to
4e19052
Compare
4e19052 to
9d88f8b
Compare
Signed-off-by: Thomas Güttler <thomas.guettler@syself.com>
9d88f8b to
f29b4c4
Compare
mbuechse
left a comment
There was a problem hiding this comment.
I think there is still potential for improvement, but let's see how this works.
Closes #1087