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

fix: validate and parse nested models properly with default_factory #1712

Merged

Conversation

PrettyWood
Copy link
Collaborator

@PrettyWood PrettyWood commented Jul 12, 2020

Change Summary

PR #1504 introduced a regression by bypassing populate_validators(),
which would skip the validation and mess with parsing of nested models with default_factory

Related issue number

closes #1710
closes #1717
closes #1722

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)

PR samuelcolvin#1504 introduced a regression by bypassing `populate_validators()`,
which would skip the validation of children in nested models
with `default_factory`

closes samuelcolvin#1710
@codecov
Copy link

@codecov codecov bot commented Jul 12, 2020

Codecov Report

Merging #1712 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #1712   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         3893      3895    +2     
  Branches       783       783           
=========================================
+ Hits          3893      3895    +2     
Impacted Files Coverage Δ
pydantic/fields.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5dbb127...e43a72b. Read the comment docs.

@PrettyWood PrettyWood changed the title fix: validate nested models with default_factory fix: validate and parse nested models properly with default_factory Jul 13, 2020
@PrettyWood PrettyWood force-pushed the fix/default-factory-validate-list branch from 52ca25a to 16e489b Compare Jul 13, 2020
@dgasmith
Copy link
Contributor

@dgasmith dgasmith commented Jul 14, 2020

Thanks for finding and fixing this so quickly! Is there anything we can do to help expedite a 1.6.1 patch release? This issue has some pretty far reaching complications for our software stack.

@samuelcolvin
Copy link
Owner

@samuelcolvin samuelcolvin commented Jul 15, 2020

thanks so much. This looks good, I've tweaked some of the tests and added another one, otherwise I think this is good to go, will merge once it passes and release a patch.

@samuelcolvin samuelcolvin merged commit e2fcab5 into samuelcolvin:master Jul 15, 2020
14 checks passed
@samuelcolvin
Copy link
Owner

@samuelcolvin samuelcolvin commented Jul 15, 2020

Thanks so much @PrettyWood for solving this so quickly and to all those who reported it.

I've just tagged a release, it should be available in pypi in a few minutes.

Sorry for the slightly delay.

@PrettyWood PrettyWood deleted the fix/default-factory-validate-list branch Jul 15, 2020
PrettyWood added a commit to PrettyWood/pydantic that referenced this issue Jul 22, 2020
PR samuelcolvin#1712 introduced a regression for forward refs in `ModelField.prepare`
as it would not return early for forward refs anymore.
Optional fields would hence have `required` set to `True`.

closes samuelcolvin#1736
samuelcolvin pushed a commit that referenced this issue Oct 8, 2020
* fix: forward ref with nested models and optional fields

PR #1712 introduced a regression for forward refs in `ModelField.prepare`
as it would not return early for forward refs anymore.
Optional fields would hence have `required` set to `True`.

closes #1736

* test: skip python 3.6 as __future__.annotations is not defined
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants