Fixes #581 #583
Fixes #581 #583
Conversation
| @@ -137,7 +137,7 @@ class DBInstance(AWSObject): | |||
| 'DBSecurityGroups': (list, False), | |||
| 'DBSnapshotIdentifier': (basestring, False), | |||
| 'DBSubnetGroupName': (basestring, False), | |||
| 'Engine': (validate_engine, True), | |||
There was a problem hiding this comment.
The use of validate_engine can still be used here instead of adding additional code in validate but change required to False.
There was a problem hiding this comment.
But if Engine isn't specified, wouldn't validate_engine fail?
There was a problem hiding this comment.
Good question. The property checks (type or validate functions) are only called if the property is specified in the class instantiation. And then the class validate function is called later on to do validation across all of the properties.
| @@ -221,6 +228,10 @@ def validate(self): | |||
| raise ValueError("AllocatedStorage must be no less than " | |||
| "1/10th the provisioned Iops") | |||
|
|
|||
| engine = self.properties.get('Engine', None) | |||
There was a problem hiding this comment.
With validate_engine used above this isn't needed.
| 'Either Engine or DBSnapshotIdentifier is required ' | ||
| 'AWS::RDS::DBInstance.' | ||
| ) | ||
|
|
There was a problem hiding this comment.
Since Engine can be optional when specifying DBSnapshotIdentifier, perhaps this could be coded:
if 'DBSnapshotIdentifier' not in self.properties:
if 'Engine' not in self.properties:
raise ValueError(
'Resource Engine required in type %s' % self.resource_type)```
|
Thank you for the PR and the additional rework. |
…loudtools#583) * Fixes cloudtools#581 * Update to verify engine later in the process * Update patch according to recommendations
No description provided.