Feature/strict schema #169
Conversation
ghost
assigned 
| class ResponseSchema(ActuallyRequireOnDumpMixin, Schema): | ||
| def __init__(self, **kwargs): | ||
| super().__init__(**kwargs) |
There was a problem hiding this comment.
Function call overhead in python is significant. It’s best practice not to introduce this extra no-op function call that reduces performance just for some IDEs that aren’t currently finding the inherited method. Can you follow up with your IDE vendor instead of adding this here?
| super().__init__(**kwargs) | ||
|
|
||
|
|
||
| class StrictSchema(RequestSchema, ResponseSchema): | ||
| pass |
There was a problem hiding this comment.
Missing docstring for public class.
Once added, the pass will be unnecessary.
| @@ -117,6 +117,30 @@ def test_required_failed_validate(self): | |||
| self.assertIn("one_of_validation", ctx.exception.messages) | |||
|
|
|||
|
|
|||
| class ActuallyStrictSchema(StrictSchema): | |||
There was a problem hiding this comment.
TestStrictSchema or MyStrictSchema? Shout out to #170.
| super().__init__(**kwargs) | ||
|
|
||
|
|
||
| class StrictSchema(RequestSchema, ResponseSchema): |
There was a problem hiding this comment.
- Better to inherit the mixin classes directly
- I wonder if there’s a better name than StrictSchema for this, since strict is overloaded with other meaning in Marshmallow v2 (complicated by how Rebar currently already always does a strict load with unstrict Marshmallow v2 schemas, i.e. raising rather than returning a value with errors in it)
|
|
|
@Sytten Thanks for working on this. It looks great, but I would echo the comments about wanting the no-op init removed and documentation updates being needed. |
|
Looks like we have some merge conflicts with this pr. I am working on cleaning up the backlog for flask rebar, so if i can help, please let me know. |
Fixes #161
This adds a new
StrictSchemawhich we should recommend for new users in #152. It's a simple combination ofRequestSchemaandResponseSchema. This is also interesting if we want to extend some functionality of marshmallow down the road.I also added a simple constructor to
ResponseSchemabecause of #137. A future improvement (if/when we drop marshmallow v2) would be to have all the keyword arguments so the IDE can pick it up and suggest them to the user. At least now it doesn't show up as an error.