fix: pydantic 2.10.x breaking change #1095

mdaffad · 2024-12-25T02:29:10Z

Changes:

Breaking change on AnyUrl type
Breaking change on pydantic_core.core_schema.no_info_plain_validator_function

Known affected test on pydantic latest release:

tests/odm/test_encoder.py::test_should_encode_pydantic_v2_url_correctly
tests/odm/test_encoder.py::test_should_be_able_to_save_retrieve_doc_with_url
tests/odm/test_fields.py::test_revision_id_not_in_schema
tests/odm/test_json_schema_generation.py::test_schema_export_of_model_with_decimal_field
tests/odm/test_json_schema_generation.py::test_schema_export_of_model_with_pydanticobjectid
tests/odm/test_json_schema_generation.py::test_schema_export_of_model_with_link
tests/odm/test_json_schema_generation.py
tests/odm/test_json_schema_generation.py::test_schema_export_of_model_with_list_link
tests/odm/test_json_schema_generation.py::test_schema_export_of_model_with_backlink
tests/odm/test_json_schema_generation.py::test_schema_export_of_model_with_list_backlink

Related issues:

Closes [BUG] Pydantic URLs broken in pydantic 2.10.3 #1084

Viicos · 2024-12-26T20:56:40Z

beanie/odm/fields.py

+                        return_schema=str_schema(),
+                        when_used="json",
+                    ),
+                )
            return json_or_python_schema(


Hello, Pydantic maintainer here.

I don't think there's a need to make a distinction between JSON and Python here. pydantic_js_input_core_schema can be specified for both, it's not an issue:

return no_info_plain_validator_function( cls._validate, json_schema_input_schema=..., serialization=..., )

As a side note, I know our logic to create custom types through __get_pydantic_core_schema__ isn't ideal (confusing pattern, need to create core schemas manually, etc). Ideally, PydanticObjectId could be defined as an annotated type:

PydanticObjectId = Annotated[ ObjectId, PlainValidator(_validate, json_schema_input_type=Annotated[str, Field(pattern="^[0-9a-f]{24}$", min_length=24, max_length=24)]), PlainSerializer(lambda v: str(v), return_type=str), ]

This way, you go through the "user facing" validators API, where json_schema_input_type is guaranteed to not change (unlike how this attribute was stored on the core schema — we had to change it in 2.10 to avoid core schema simplification errors).

Thanks for the suggestion. I updated on the plain validator function. But for the

PydanticObjectId = Annotated[ ObjectId, PlainValidator(_validate, json_schema_input_type=Annotated[str, Field(pattern="^[0-9a-f]{24}$", min_length=24, max_length=24)]), PlainSerializer(lambda v: str(v), return_type=str), ]

I have noticed there are some breaking changes that would require significant modifications to the codebase since it changes the checker for PydanticObjectId as typing.Annotated and not a normal class. One of the example is:

def extract_id_class(annotation) -> Type[Any]: if get_origin(annotation) is not None: try: annotation = next( arg for arg in get_args(annotation) if arg is not type(None) ) except StopIteration: annotation = None if inspect.isclass(annotation): return annotation raise ValueError("Unknown annotation: {}".format(annotation))

Yes indeed this will most likely break things (and the Annotated form also has some drawbacks). And it seems that beanie expects PydanticObjectId to be instantiable, which is not going to be the case with Annotated :/

…10.x-breaking-change

Viicos · 2024-12-27T09:07:02Z

beanie/odm/utils/pydantic.py

@@ -1,9 +1,22 @@
 from typing import Any, Type

 import pydantic
+from packaging.specifiers import SpecifierSet


You'll need to switch back to the pydantic.VERSION manual parsing as packacing is a 3rd party dependency, and currently not included in the requirements.

I agree here, lets not introduce packaging as a dependency.

staticxterm

Hi, thank you for your contribution.
Looks good in general, no further major comments apart from what @Viicos already commented.

staticxterm · 2024-12-27T20:23:03Z

.github/workflows/github-actions-tests.yml

+on:
+  pull_request:
+  push:


I'm not so sure about this seemingly unrelated change.
Won't this run multiple jobs for a "PR open" and a "branch push"?

Hi thanks to check this PR.
I used it in for CI tests on current branch before make the PR. That part has been removed now.

staticxterm · 2024-12-27T20:53:21Z

beanie/odm/utils/pydantic.py

@@ -1,9 +1,22 @@
 from typing import Any, Type

 import pydantic
+from packaging.specifiers import SpecifierSet


I agree here, lets not introduce packaging as a dependency.

staticxterm

No other comments apart from the version check one, looks good.

staticxterm · 2024-12-28T12:43:43Z

beanie/odm/utils/pydantic.py

+IS_PYDANTIC_V2 = int(pydantic.VERSION.split(".")[0]) >= 2
+IS_PYDANTIC_V2_10 = (
+    IS_PYDANTIC_V2 and int(pydantic.VERSION.split(".")[1]) >= 10
+) or int(pydantic.VERSION.split(".")[0]) > 2


I would remove this "or" part. Any other major Pydantic version would not necessarily be compatible with Beanie and I think we should not bet on this.
Lets make this check do what it says in the name, to explicitly check for Pydantic v2.10.x (or above, constrained to v2).

staticxterm · 2024-12-28T14:34:01Z

Sorry, @mdaffad, one other thing also popped to mind. Lets also include latest Pydantic v2.10.4 in our CI testing as well?
Please add it to .github/workflows/github-actions-tests.yml

mdaffad · 2024-12-28T14:49:30Z

It seems version 2.10.4 from matrix dropped after this run: https://github.com/BeanieODM/beanie/actions/runs/12488481332/workflow. I will re-add it.

Riverfount

LGTM

mdaffad added 2 commits December 24, 2024 17:41

fix: breaking change with pydantic 2.10.x

4f423b6

chore: test on gha

fc2b38a

adeelsohailahmed requested a review from staticxterm December 25, 2024 19:08

Viicos reviewed Dec 26, 2024

View reviewed changes

mdaffad added 2 commits December 27, 2024 10:35

fix: unify python and json validator schema

ee11cd9

Merge branch 'main' of github.com:mdaffad/beanie into fix-pydantic-2.…

b045335

…10.x-breaking-change

Viicos reviewed Dec 27, 2024

View reviewed changes

staticxterm requested changes Dec 27, 2024

View reviewed changes

mdaffad added 2 commits December 28, 2024 07:46

fix: revert version checking logic

848d14c

fix: drop on push event on github actions

ce4ab1f

mdaffad requested a review from staticxterm December 28, 2024 12:03

staticxterm requested changes Dec 28, 2024

View reviewed changes

fix: marker logic for pydantic v2.10

ae0d0a7

mdaffad requested a review from staticxterm December 28, 2024 14:15

staticxterm previously approved these changes Dec 28, 2024

View reviewed changes

staticxterm requested a review from a team December 28, 2024 14:22

fix: add test pydantic 2.10.4

04bdc8d

mdaffad dismissed staticxterm’s stale review via 04bdc8d December 28, 2024 14:50

mdaffad requested a review from staticxterm December 28, 2024 15:22

Riverfount approved these changes Dec 28, 2024

View reviewed changes

adeelsohailahmed requested review from Viicos and a team and removed request for Viicos December 28, 2024 18:05

staticxterm approved these changes Dec 28, 2024

View reviewed changes

adeelsohailahmed requested a review from Viicos December 31, 2024 18:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: pydantic 2.10.x breaking change #1095

fix: pydantic 2.10.x breaking change #1095

mdaffad commented Dec 25, 2024 •

edited

Loading

Viicos Dec 26, 2024 •

edited

Loading

mdaffad Dec 27, 2024

Viicos Dec 27, 2024

Viicos Dec 27, 2024

staticxterm Dec 27, 2024

staticxterm left a comment

staticxterm Dec 27, 2024

mdaffad Dec 28, 2024

staticxterm Dec 27, 2024

staticxterm left a comment

staticxterm Dec 28, 2024

staticxterm commented Dec 28, 2024

mdaffad commented Dec 28, 2024

Riverfount left a comment

fix: pydantic 2.10.x breaking change #1095

Are you sure you want to change the base?

fix: pydantic 2.10.x breaking change #1095

Conversation

mdaffad commented Dec 25, 2024 • edited Loading

Viicos Dec 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

staticxterm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

staticxterm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

staticxterm commented Dec 28, 2024

mdaffad commented Dec 28, 2024

Riverfount left a comment

Choose a reason for hiding this comment

mdaffad commented Dec 25, 2024 •

edited

Loading

Viicos Dec 26, 2024 •

edited

Loading