-
Notifications
You must be signed in to change notification settings - Fork 305
Validate default values at schema definition #1593
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #1593 will not alter performanceComparing Summary
|
please review |
Thank you! The problem this addresses has been a persistent thorn in our sides. Would love to see this released. |
This prevents issues where the node is defined with an invalid default value, which would guarantee an error during a ser/de roundtrip. - Upstream issue requesting this functionality be built-in to pydantic: pydantic/pydantic#8722 - Upstream PR that implements the functionality: pydantic/pydantic-core#1593
This prevents issues where the node is defined with an invalid default value, which would guarantee an error during a ser/de roundtrip. - Upstream issue requesting this functionality be built-in to pydantic: pydantic/pydantic#8722 - Upstream PR that implements the functionality: pydantic/pydantic-core#1593
This prevents issues where the node is defined with an invalid default value, which would guarantee an error during a ser/de roundtrip. - Upstream issue requesting this functionality be built-in to pydantic: pydantic/pydantic#8722 - Upstream PR that implements the functionality: pydantic/pydantic-core#1593
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this seems good to me, and I'm sorry the review took so long.
Just one thing I would like to check is the choice of bitflags, I think we can just have a plain enum.
Once this is merged we will need to document it in pydantic
.
bitflags::bitflags! { | ||
#[derive(Debug, Clone)] | ||
struct ValidateDefaultFlag: u8 { | ||
const NEVER = 0; | ||
const INIT = 0x01; | ||
const DEFINITION = 0x02; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needs to be a bitflag, can just be an enum
?
Change Summary
Extended option
validate_default
so that it's possible to validate default values when a model schema is being built.This option now can be a boolean or 3 possible string values:
'never'
,'definition'
, or'init'
.never
: no validation for default values at all. This is the same asFalse
.init
: validation at model instantiation. This is the same asTrue
.definition
: validation at model definition.The behaviour of
True
/False
does not change with this PR.Note that even though
init
anddefinition
are not mutually exclusive and I implemented these as flags that can co-exist, I decided not to add another option, e.g.always
, that cover both flags because if a validation is raised at model definition, it's pretty much impossible to trigger such error at model instantiation because the model does not built at all and therefore we don't really needalways
.Related issue number
Part of pydantic/pydantic#8722
Checklist
pydantic-core
(except for expected changes)Selected Reviewer: @davidhewitt