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

Allow to keep the types as they were and just add a default value #39

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

KrilleGH
Copy link

It's still possible to omit all unchanged fields in PATCH requests, but for fields that are given Pydantic will reject data that specifies an explicit null for fields that aren't optional in the full model.

This is useful when using the package with SQLModel. Without this feature the client could try to assign a NULL value to a column with a NOT NULL constraint, which would raise an IntegrityError.

@ddanier
Copy link
Member

ddanier commented Nov 27, 2024

I understand the issue here, but I don't think this will work for all cases. Pydantic might have issues with the default value not matching the type definition.

Could you add some tests to ensure this all works? ...including this None does not match the type definition thing?

It's still possible to omit all unchanged fields in PATCH requests, but
for fields that are given Pydantic will reject data that specifies an
explicit null for fields that aren't optional in the full model.

This is useful when using the package with SQLModel. Without this
feature the client could try to assign a NULL value to a column with a
NOT NULL constraint, which would raise an IntegrityError.
@KrilleGH
Copy link
Author

KrilleGH commented Dec 5, 2024

Hey @ddanier,

here are some tests for the feature. Is that how you imagined?

It seems Pydantic V1 handles default values differently so _field_get_default() doesn't return the expected values. Do you happen to know how to fix that? Otherwise we could also skip the test for Pydantic V1, it doesn't seem so important.

What is more important is that test_validate_fail doesn't raise on Pydantic V1, is this something where this version behaved differently?

Is there special need to remain compatible with Pydantic V1? Otherwise this feature could just be supported on V2.

Cheers
Christian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants