-
Notifications
You must be signed in to change notification settings - Fork 6
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.
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
Refactoring validate() to handle resource data collections #5
Conversation
Now yields either a validated resource or a resource generator callable, depending on the value of new kwarg `collection`.
Example use for relationship data collections:
|
@brosner sure would be nice to get this merged! |
flake8 is unhappy with Resource.serialize.
Changes Unknown when pulling 29830b2 on grahamu:all-purpose-validate into * on pinax:master*. |
@contextlib.contextmanager needed all yield statements inside a try block in order to properly catch validation errors arising from resource manipulation. For instance, resource.save() may raise attribute validation problems that resource.populate() does not.
Changes Unknown when pulling 4ba2a87 on grahamu:all-purpose-validate into * on pinax:master*. |
Needs four types of tests. |
for resource_data in data["data"]: | ||
yield self.validate_resource(resource_class, resource_data, obj) | ||
|
||
yield gen_func |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we went the wrong way here. The generator idea was a little ill-conceived on my part. However, I think the fix is very easy and even cleaner. I am changing my mind after seeing the resulting code for the validate
caller. Having to call the generator is a bit ugly IMO.
The new idea is to simply change this line to:
yield (self.validate_resource(resource_class, resource_data, obj) for resource_data in data["data"])
The change to the caller is to remove the function call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, much cleaner code both ways. Thanks.
Changes Unknown when pulling d1339c1 on grahamu:all-purpose-validate into * on pinax:master*. |
Now
validate()
yields either a validated resource or a resource generator callable,depending on the value of new kwarg
collection
.