Validate Only and Exclude #826
Conversation
marshmallow/schema.py
Outdated
| else: | ||
| message = '{0} is not a valid field for {1}.' | ||
| message = message.format(invalid_names, self.__class__.__name__) | ||
| raise AttributeError(message) |
There was a problem hiding this comment.
AttributeError doesn't feel like the correct error. From the docs.
Raised when an attribute reference (see Attribute references) or assignment fails.
I think ValueError is more appropriate.
There was a problem hiding this comment.
That makes sense. I was thinking of fields as schema class attributes, but that isn't really how they are used.
marshmallow/utils.py
Outdated
| return ' and '.join(values) | ||
| if len(values) == 1: | ||
| return values[0] | ||
| return 'None' |
There was a problem hiding this comment.
Would it make more sense to raise an error instead of return 'None' here? We shouldn't ever hit this, but it would be incorrect to get 'None is not a valid field for MySchema'.
There was a problem hiding this comment.
As a generic utility it seemed like a logical representation of an empty list. It is currently impossible to hit this line from Schema._update_fields().
Maybe we don't need to format the message as a single string though. Developers seem to be just fine with lists and dictionaries as the "message" value for validation exceptions. Any preference for the message structure?
There was a problem hiding this comment.
Maybe we don't need to format the message as a single string though. Developers seem to be just fine with lists and dictionaries as the "message" value for validation exceptions. Any preference for the message structure?
Yep. I wouldn't bother creating a string.
if invalid_fields:
raise AttributeError("Invalid fields for {0}: {1}".format(self.__class__.__name__, invalid_fields))Validation errors may be exposed to end users (like when using webargs) or even parsed by API clients, so they should be consistent, but this is just meant to be seen by the developer after a typo, so don't overthink it.
marshmallow/schema.py
Outdated
| :param tuple|list only: Whitelist of the declared fields to select when | ||
| instantiating the Schema. If None, all fields are used. Nested fields | ||
| can be represented with dot delimiters. | ||
| :param tuple|list exclude: Blacklist of of the declared fields to exclude |
There was a problem hiding this comment.
Blacklist of of the declared fields
"of" is repeated
marshmallow/schema.py
Outdated
|
|
||
| if invalid_fields: | ||
| invalid_names = ['"{}"'.format(name) for name in invalid_fields] | ||
| invalid_names = utils.iterable_to_string(invalid_names) |
There was a problem hiding this comment.
(Pylint nazi inside)
Reusing variable name is considered bad practice (R0204 (redefined-variable-type)).
You could append _list or _str to one of them, or just go for the one-liner equivalent.
deckar01
force-pushed
the
636-validate-only
branch
from
98b2cda
to
24b921d
Compare
…rializaton Only field was not consistent with its treat of fields and declared fields. To make it consistent fieldnames passed in as only not present in Schema will be ignored. Two previous tests which were checking for errors with invalid only fields were dropped as a result. Documentation has also been updated to explicitly state that invalid fieldnames will be ignored Documentation has also been updated to explicitly state that invalid fieldnames will be ignored.
deckar01
force-pushed
the
636-validate-only
branch
from
24b921d
to
84e9a84
Compare
|
@deckar01 Would you mind updating the changelog and possibly the upgrading docs as well? LMK if you don't have time right now; I can do it when I have some time this weekend or next week. |
Validate
onlyandexcludeagainst the declared fields to give developers feedback when they use invalid field names in the schema configuration.Fixes #636