-
Notifications
You must be signed in to change notification settings - Fork 46
Fix/return validation errors instead of db errors #1923
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
Conversation
…hat override
_deserialize() call super()._deserialize()
Signed-off-by: F.N. Claessen <felix@seita.nl>
…t override
_deserialize() call super()._deserialize()
Signed-off-by: F.N. Claessen <felix@seita.nl>
…deserialize method Signed-off-by: F.N. Claessen <felix@seita.nl>
…lass methods Signed-off-by: F.N. Claessen <felix@seita.nl>
…methods Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…er fields, too Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…hmallow-field-subclass-deserialization
nhoening
left a comment
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.
Nice cleanup work, and good use of the inspect and importlib libraries to test this :)
Good for UX. And arguably this makes FlexMeasures a bit more secure as it leaks no internal information this way (well, the db structure was open source anyway).
I have just one request: Run make update-docs
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Description
load_default=Nonetorequired=Falseso that_serializemethods no longer need to pass throughNonevalues (apispec caught this)_serializeand_deserializemethodsJSONMarshmallow field was defined twice -> refactoreddocumentation/changelog.rstdocumentation/api/changelog.rstdocumentation/cli/changelog.rstLook & Feel
Now matches expected responses detailed in #1922.
How to test
The new
test_all_custom_fields_call_super_deserializemakes sure that all fields mapping to a db object callssuper()._deserialize, which checks that the value is actually an integer ID.Further Improvements
The test uses a util method (
deserialize_calls_super) with a switch (and_queries_db). If we set this toTruewe would also cover all fields that do not themselves query the db as part of deserializing. I tuned this off for now, because we have various fields that worked around this bug in one way or another, using their own validation logic rather than that of their superclass Marshmallow field. For instance:could instead have been:
in which case the ValidationError would become slightly less informative.
In this particular case, the replacement would probably still be acceptable, but some others have more explicit error messages right now.
Related Items
Closes #1922.