Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve double validation and fix some property datatypes #1179

Merged
merged 4 commits into from
Oct 29, 2018
Merged

improve double validation and fix some property datatypes #1179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf05f1d
improve double validation and fix some property datatypes
robertaistleitner Oct 18, 2018
9914444
please the linter
robertaistleitner Oct 18, 2018
1af6282
Merge branch 'master' of https://github.com/cloudtools/troposphere in…
robertaistleitner Oct 23, 2018
46e2d77
adopt use of floatingpoint to double due to merge of master
robertaistleitner Oct 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions troposphere/apigateway.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from . import AWSObject, AWSProperty
from .validators import (
boolean, floatingpoint, integer_range, json_checker, positive_integer
boolean, double, integer_range, json_checker, positive_integer
)


Expand Down Expand Up @@ -86,7 +86,7 @@ class CanarySetting(AWSProperty):

props = {
"DeploymentId": (basestring, False),
"PercentTraffic": ([floatingpoint], False),
"PercentTraffic": ([double], False),
"StageVariableOverrides": (dict, False),
"UseStageCache": (boolean, False),
}
Expand All @@ -110,7 +110,7 @@ class ClientCertificate(AWSObject):
class DeploymentCanarySettings(AWSProperty):

props = {
"PercentTraffic": ([floatingpoint], False),
"PercentTraffic": ([double], False),
"StageVariableOverrides": (dict, False),
"UseStageCache": (boolean, False),
}
Expand Down
4 changes: 2 additions & 2 deletions troposphere/applicationautoscaling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from . import AWSObject, AWSProperty
from .validators import boolean, floatingpoint, integer, positive_integer
from .validators import boolean, double, integer, positive_integer


class ScalableTargetAction(AWSProperty):
Expand Down Expand Up @@ -84,7 +84,7 @@ class TargetTrackingScalingPolicyConfiguration(AWSProperty):
(PredefinedMetricSpecification, False),
'ScaleInCooldown': (positive_integer, False),
'ScaleOutCooldown': (positive_integer, False),
'TargetValue': (floatingpoint, True),
'TargetValue': (double, True),
}


Expand Down
5 changes: 3 additions & 2 deletions troposphere/cloudwatch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
# See LICENSE file for full license.

from . import AWSObject, AWSProperty
from .validators import boolean, exactly_one, json_checker, positive_integer
from .validators import (boolean, double, exactly_one, json_checker,
positive_integer)


class MetricDimension(AWSProperty):
Expand Down Expand Up @@ -33,7 +34,7 @@ class Alarm(AWSObject):
'OKActions': ([basestring], False),
'Period': (positive_integer, True),
'Statistic': (basestring, False),
'Threshold': (basestring, True),
'Threshold': (double, True),
'TreatMissingData': (basestring, False),
'Unit': (basestring, False),
}
Expand Down
4 changes: 2 additions & 2 deletions troposphere/ec2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from . import AWSHelperFn, AWSObject, AWSProperty, Tags
from .validators import (
boolean, exactly_one, integer, integer_range, floatingpoint,
boolean, exactly_one, integer, integer_range, double,
network_port, positive_integer, vpn_pre_shared_key, vpn_tunnel_inside_cidr,
vpc_endpoint_type
)
Expand Down Expand Up @@ -765,7 +765,7 @@ class LaunchTemplateOverrides(AWSProperty):
'InstanceType': (basestring, False),
'SpotPrice': (basestring, False),
'SubnetId': (basestring, False),
'WeightedCapacity': (floatingpoint, False)
'WeightedCapacity': (double, False)
}


Expand Down
4 changes: 2 additions & 2 deletions troposphere/emr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from . import AWSObject, AWSProperty, AWSHelperFn, Tags
from .validators import (
boolean, integer, positive_integer, floatingpoint, defer
boolean, integer, positive_integer, double, defer
)


Expand Down Expand Up @@ -197,7 +197,7 @@ def validate(self):
if adjustment_type == CHANGE_IN_CAPACITY:
integer(scaling_adjustment)
elif adjustment_type == PERCENT_CHANGE_IN_CAPACITY:
floatingpoint(scaling_adjustment)
double(scaling_adjustment)
f = float(scaling_adjustment)
if f < 0.0 or f > 1.0:
raise ValueError(
Expand Down
8 changes: 4 additions & 4 deletions troposphere/glue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# See LICENSE file for full license.

from . import AWSObject, AWSProperty
from .validators import boolean, floatingpoint, integer_range, positive_integer
from .validators import boolean, double, integer_range, positive_integer


class GrokClassifier(AWSProperty):
Expand Down Expand Up @@ -194,7 +194,7 @@ class ConnectionsList(AWSProperty):

class ExecutionProperty(AWSProperty):
props = {
'MaxConcurrentRuns': (floatingpoint, False),
'MaxConcurrentRuns': (positive_integer, False),
}


Expand All @@ -209,14 +209,14 @@ class Job(AWSObject):
resource_type = "AWS::Glue::Job"

props = {
'AllocatedCapacity': (floatingpoint, False),
'AllocatedCapacity': (double, False),
'Command': (JobCommand, True),
'Connections': (ConnectionsList, False),
'DefaultArguments': (dict, False),
'Description': (basestring, False),
'ExecutionProperty': (ExecutionProperty, False),
'LogUri': (basestring, False),
'MaxRetries': (floatingpoint, False),
'MaxRetries': (double, False),
'Name': (basestring, False),
'Role': (basestring, True),
}
Expand Down
4 changes: 2 additions & 2 deletions troposphere/validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ def integer_list_item_checker(x):
return integer_list_item_checker


def floatingpoint(x):
def double(x):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than replacing it entirely, maybe we should keep a copy of floatingpoint around for backwards compatibility. Two things we could do:

  1. Just do the update here, then set floatingpoint to be floatingpoint = double here as well.
  2. Have floatingpoint call double, but include a warning that floatingpoint will be deprecated soon.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for sending the comment multiple times - blame Github for it 😉

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done ✔️

try:
float(x)
except (ValueError, TypeError):
raise ValueError("%r is not a valid float" % x)
raise ValueError("%r is not a valid double" % x)
else:
return x

Expand Down