Skip to content
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

Require fields for non-nullable properties. #40

Merged
merged 3 commits into from Oct 27, 2015

Conversation

@jmcarp
Copy link
Contributor

commented Oct 26, 2015

This is helpful for the common use case of creating a new record from
data: if required fields are missing, we should throw an error. For
partial updates, users would pass partial=True to the ModelSchema
constructor as usual.

jmcarp added some commits Oct 26, 2015

Require fields for non-nullable properties.
This is helpful for the common use case of creating a new record from
data: if required fields are missing, we should throw an error. For
partial updates, users would pass `partial=True` to the `ModelSchema`
constructor as usual.
Pass `partial` as needed in tests.
Also temporarily install marshmallow from dev--didn't realize partial
was such a new addition.

@jmcarp jmcarp force-pushed the jmcarp:require-not-nullable branch from e1ee46f to 98984e8 Oct 26, 2015

@sloria

This comment has been minimized.

Copy link
Member

commented Oct 26, 2015

I'm going to cut the marshmallow 2.2.0 release before merging this, so we don't have to change .travis.yml

@sloria

This comment has been minimized.

Copy link
Member

commented Oct 26, 2015

Though, I suppose the minimum supported marshmallow version would need to be bumped up to 2.2.0

@sloria

This comment has been minimized.

Copy link
Member

commented Oct 26, 2015

Actually, this doesn't technically break backwards compat with marshmallow 2.0.0 (partial is only used in the tests). Can you add a conditional for the assertions that use partial?

@sloria

This comment has been minimized.

Copy link
Member

commented Oct 26, 2015

Btw, marshmallow 2.2.0 is released, so you should be able to do

marshmallow_version_info = tuple(map(int, marshmallow.__version__.split('.')))
if marshmallow_version_info >= (2, 2, 0):
    #...
Deserialize complete data in tests.
Avoid the need to pass `partial` and inspect version number in the
tests: the tests are responsible for checking deserialization behavior,
not required fields or partial loading.
@jmcarp

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2015

IMO it's simpler just to deserialize complete objects in the tests, so we don't have to worry about partial marshmallow version. Patch updated.

sloria added a commit that referenced this pull request Oct 27, 2015

Merge pull request #40 from jmcarp/require-not-nullable
Require fields for non-nullable properties.

@sloria sloria merged commit c692177 into marshmallow-code:dev Oct 27, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.