improve double validation and fix some property datatypes #1179
Conversation
- replace `floatingpoint` validator with more valid `double` validator - this is also the naming used in AWS CloudFormation - update CloudWatch Alarm Threshold datatype to double (according to [documentation](https://docs.aws.amazon.com/de_de/AWSCloudFormation/latest/UserGuide/aws-properties-cw-alarm.html#cfn-cloudwatch-alarms-threshold)) - update Glue ExecutionProperty MaxConcurrentRuns datatype to positive_integer (according to [documentation](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-glue-job-executionproperty.html#cfn-glue-job-executionproperty-maxconcurrentruns))
| @@ -53,11 +53,11 @@ def integer_list_item_checker(x): | |||
| return integer_list_item_checker | |||
|
|
|||
|
|
|||
| def floatingpoint(x): | |||
| def double(x): | |||
There was a problem hiding this comment.
Rather than replacing it entirely, maybe we should keep a copy of floatingpoint around for backwards compatibility. Two things we could do:
- Just do the update here, then set floatingpoint to be
floatingpoint = doublehere as well. - Have floatingpoint call
double, but include a warning thatfloatingpointwill be deprecated soon.
What do you think?
There was a problem hiding this comment.
Do we expect floatingpoint to be used outside of this package? In this case I will change it of course, although if this is not the case I'd rather clean it up right away since nothing like a floatingpoint is found throughout the AWS CloudFormation documentation, right?
Ad 1. This would show a wrong error message stating that a floatingpoint is expected.
Ad 2. See first paragraph, if there's external references to it this would probably be the best solution.
There was a problem hiding this comment.
Sorry for sending the comment multiple times - blame Github for it 😉
There was a problem hiding this comment.
Good point - it's unlikely it's being used in custom resources, so this is probably fine. One last thing: can you please merge master into this branch, and just make sure there are no remaining cases? We've had a few merges, and some may have used floatingpoint.
Thanks - I'll accept/merge this as soon as you have a chance to merge master!
|
Thanks @robertaistleitner ! |
…#1179) * improve double validation and fix some property datatypes - replace `floatingpoint` validator with more valid `double` validator - this is also the naming used in AWS CloudFormation - update CloudWatch Alarm Threshold datatype to double (according to [documentation](https://docs.aws.amazon.com/de_de/AWSCloudFormation/latest/UserGuide/aws-properties-cw-alarm.html#cfn-cloudwatch-alarms-threshold)) - update Glue ExecutionProperty MaxConcurrentRuns datatype to positive_integer (according to [documentation](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-glue-job-executionproperty.html#cfn-glue-job-executionproperty-maxconcurrentruns)) * please the linter * adopt use of floatingpoint to double due to merge of master
fixes #812 + improves validator consistency with AWS naming
floatingpointvalidator with more validdoublevalidator - this is also the naming used in AWS CloudFormation