Change how we handle defaults. #171
Conversation
jnb
commented
Apr 19, 2017
jnb
commented
- Only allow a None to be returned for an optional field if that field is x-nullable.
- Instantiate a missing field to its default value if provided.
bravado_core/unmarshal.py
Outdated
|
|
||
| result = {} | ||
| for k, v in iteritems(object_value): | ||
| prop_spec = get_spec_for_prop( | ||
| swagger_spec, object_spec, object_value, k) | ||
| if v is None and k not in required_fields and prop_spec: |
There was a problem hiding this comment.
I'd be OK to revert this hunk if you think it's not worth the amount of pain it might cause?
There was a problem hiding this comment.
I think the change makes sense conceptually, not sure how many people depend on being allowed to send None for optional fields though.
bravado_core/unmarshal.py
Outdated
|
|
||
| result = {} | ||
| for k, v in iteritems(object_value): | ||
| prop_spec = get_spec_for_prop( | ||
| swagger_spec, object_spec, object_value, k) | ||
| if v is None and k not in required_fields and prop_spec: |
There was a problem hiding this comment.
I think the change makes sense conceptually, not sure how many people depend on being allowed to send None for optional fields though.
| # This should fail because the field is not nullable | ||
| with pytest.raises(SwaggerMappingError) as excinfo: | ||
| unmarshal_object(empty_swagger_spec, content_spec, value) | ||
| assert 'is a required value' in str(excinfo.value) |
There was a problem hiding this comment.
This is a misleading error message, as the field is not required. We just don't allow None as value anymore.
|
OK, I scoped this down a bit. Now the behavior is just to add a default field value in case of a missing field. If there's isn't a default field specified, then we add a None (as before). |
|
I've released version 4.7.3 with this change. |
