Plat 252 change exporter #2
Conversation
There was a problem hiding this comment.
Looks good to me.
As said in the comments, I'd play it safer when dealing with various maps with possibly missing keys.
But the original authors don't bother so 🤷
When it comes to configuration, I can only trust you.
You said it works, I'm ok with this.
We won't add tests here anyway.
| boto3==1.28.62 | ||
| botocore==1.31.65 | ||
| boto3==1.35.8 | ||
| botocore==1.35.8 |
There was a problem hiding this comment.
You said you wanted to put this in another PR : why ?
Just to split the chores ? Or does it impact the actual code in any way ?
There was a problem hiding this comment.
I don't try this lib with IRSA on AWS.
| @@ -62,20 +49,41 @@ def validate_configs(config): | |||
| logging.error("All the target AWS accounts should have the same set of keys (labels)!") | |||
| sys.exit(1) | |||
|
|
|||
| for config_metric in config["metrics"]: | |||
|
|
|||
| if config_metric["group_by"]["enabled"]: | |||
There was a problem hiding this comment.
This may fail with a runtime error if keys are missing.
Maybe use config_metric.get("group_by", None) and handle error case ?
Or put the whole block on a try/except and handle the KeyError ?
There was a problem hiding this comment.
Also, I see the other authors don't care about this so feel free to ignore me on this.
No description provided.