-
-
Notifications
You must be signed in to change notification settings - Fork 629
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
unknown parameter passed to load method is not popagated to nested schema field. #1428
Comments
Discussed in #980. We decided that |
When deserializing a data structure with the load method, the *unknown* was not propagated to the loading of nested data structures. As result, if a unknown field was present into a nested data structure a ValidationError was raised even if the load methd was called with *unknown=EXCLUDE*. This commit ensures that this parameter is now propagated also to the loading of nested data structures. fixes marshmallow-code#1428
@lafrech Thank you for the link. In this case only the |
When deserializing a data structure with the load method, the *unknown* was not propagated to the loading of nested data structures. As result, if a unknown field was present into a nested data structure a ValidationError was raised even if the load methd was called with *unknown=EXCLUDE*. This commit ensures that this parameter is now propagated also to the loading of nested data structures. fixes marshmallow-code#1428
When deserializing a data structure with the load method, the *unknown* was not propagated to the loading of nested data structures. As result, if a unknown field was present into a nested data structure a ValidationError was raised even if the load methd was called with *unknown=EXCLUDE*. This commit ensures that this parameter is now propagated also to the loading of nested data structures. fixes marshmallow-code#1428
I think this proposed change would make it too easy for developers to implicitly opt into undesired behavior. I don't think there is any precedent for method args behaving this much differently than their If you want to change the default globally, you can with minimal code. See #1367. You could detect the context and set the global default before the schemas are declared. If you need to apply this transformation on certain schemas, I would recommend solving this use case with a utility method in your project that recursively walks through the schema fields and sets the def set_unknown_all(schema, unknown):
schema.unknown = unknown
for field in schema.fields.values():
if isinstance(field, Nested):
set_unknown_all(field.schema, unknown)
return schema |
@deckar01 When you call the load method you already have the possibility to provide your specific 'unknown' paramater that takes precedence on the one defined into your Meta counterpart. That give you the ability to control this behavior at load time without having to modify your schema. This behavior is useful when you want to validate/parse json provided by external systems without having to define the full schema. At the same time it's not because I want to ignore these unknown fields when loaded from external systems that I want to ignore these when loaded from within my code. Your proposed utility method will alter the schema definition; that's exactly what I want to avoid. |
If you set the Overriding Another consideration is that this proposal would be a breaking change. It could not be released until the next major version. It should be possible to satisfy your use case with a small amount of utility code now.
If you pass it a schema instance it should be fine. schema = Schema()
permissive_schema = set_unknown_all(Schema(), INCLUDE) |
I don't see any conclusion on this topic in the context of the load method. The discussion is still open with pros and cons and it seems that I'm not the only one thinking that it should be possible to propagate this parameter when loading...
Unwitting developers are foot-guns. These developers don't use Marshmallow at all... 😏 |
It's not fine since the value of the 'unknown' parameter specified at class level definition is lost in the instance. (I can transform this method to become a decorator ...) |
The changelog entry, including credit to prior related work, covers the closed issues and describes how `propagate_unknown` is expected to behave. Inline documentation attempts to strike a balance between clarify and brevity. closes marshmallow-code#1428, marshmallow-code#1429, marshmallow-code#1490, marshmallow-code#1575
The changelog entry, including credit to prior related work, covers the closed issues and describes how `propagate_unknown` is expected to behave. Inline documentation attempts to strike a balance between clarify and brevity. closes marshmallow-code#1428, marshmallow-code#1429, marshmallow-code#1490, marshmallow-code#1575
The changelog entry, including credit to prior related work, covers the closed issues and describes how `propagate_unknown` is expected to behave. Inline documentation attempts to strike a balance between clarify and brevity. closes marshmallow-code#1428, marshmallow-code#1429, marshmallow-code#1490, marshmallow-code#1575
Current behavior
When we deserialize a data structure containing a nested data structure by using the
load
method, theunknown
parameter is without effect on the nested data structure and a error is raised if an unknown field is present into the nested data structureExpected behavior:
unknown
parameter into load method is propagated to the the load of all nested data structure.Functional context:
when processing a REST call in a server, it is often useful to be able to ignore unknown fields in the json provided during the deserialization process. This does not mean that we want these fields to be ignored in all contexts. The use of the
unknown
parameter of theload
method allows this use case. Unfortunately, it does not apply to the nested field.The text was updated successfully, but these errors were encountered: