Provide better type checking for values in from_dict #587
Conversation
This validates properties & sub-properties when using from_dict.
| props = {} | ||
| for prop_name, value in kwargs.items(): | ||
| try: | ||
| prop_attrs = cls.props[prop_name] |
There was a problem hiding this comment.
cls.props could be missing too
There was a problem hiding this comment.
I don't think it can - this is on AWSBaseObject, and AWSBaseObject sets up self.props in its init.
There was a problem hiding this comment.
oh you're right, this is the cls object. i was thinking of the function you had before that took any class
| raise AttributeError("Object type %s does not have a " | ||
| "%s property." % (cls.__name__, | ||
| prop_name)) | ||
| prop_type, required = prop_attrs |
There was a problem hiding this comment.
maybe just prop_type = prop_attrs[0] since you don't use required?
| "list." % prop_name) | ||
| new_value = [] | ||
| for v in value: | ||
| new_value.append(prop_type[0]._from_dict(**v)) |
There was a problem hiding this comment.
what if v isn't a dict? it seems like this needs the same check for aws object
| except TypeError: | ||
| pass | ||
| if is_aws_object: | ||
| value = prop_type._from_dict(**kwargs[prop_name]) |
There was a problem hiding this comment.
what if kwargs[prop_name] isn't a dict?
|
Updated to address @mhahn 's review, with another test. |
| except TypeError: | ||
| pass | ||
| if is_aws_object: | ||
| v = kwargs[prop_name] |
There was a problem hiding this comment.
not a huge deal, but you already define value = kwargs[prop_name] above, i would just reuse that here
| value = prop_type._from_dict(**v) | ||
|
|
||
| if isinstance(prop_type, list): | ||
| if not isinstance(kwargs[prop_name], list): |
There was a problem hiding this comment.
this could just be value that you pulled out aboe
| new_value = [] | ||
| for v in value: | ||
| if not isinstance(v, collections.Mapping): | ||
| raise ValueError( |
There was a problem hiding this comment.
how would this work if this was a list of role names? does this always have to be a list of mapping types?
* Provide better type checking for values in from_dict This validates properties & sub-properties when using from_dict. * Add more test for helperfn's and invalid props * test invalid sub-property definitions * More updates, thx for review @mhahn
This validates properties & sub-properties when using from_dict.