Skip to content

Adds a pydantic V2 compatibility layer #218

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

Merged
merged 2 commits into from
Aug 28, 2023

Conversation

febus982
Copy link
Contributor

@febus982 febus982 commented Aug 28, 2023

Fixes #215

Changes

Adds a compatibility layer allowing to install the SDK together with pydantic v2, while still using the v1 implementation. Further improvements would be a native pydantic v2 implementation.

One line description for the changelog

Allows to install the SDK together with pydantic v2.

  • Tests pass
  • Appropriate changes to README are included in PR

@febus982 febus982 force-pushed the pydantic-v2-compatibility branch from 7212f67 to 329a99b Compare August 28, 2023 10:45
Signed-off-by: Federico Busetti <729029+febus982@users.noreply.github.com>
@febus982 febus982 force-pushed the pydantic-v2-compatibility branch from 329a99b to 13a61ae Compare August 28, 2023 11:09
@duglin
Copy link
Contributor

duglin commented Aug 28, 2023

@febus982
Copy link
Contributor Author

There’s a typing issue, I think it’s safe to ignore it, is it fine to ignore it? (don't want to trigger too many minutes on workers)

I have started working on the native Pydantic V2 implementation.

  • would it be enough to match the existing tests for v1?
  • there are notes about serialization/deserialization performance issues in the current implementation. Pydantic V2 should bring great performance improvements. Is there any performance testing configured that we can run?

@xSAVIKx xSAVIKx self-requested a review August 28, 2023 15:10
@xSAVIKx
Copy link
Member

xSAVIKx commented Aug 28, 2023

@febus982, thx for the PR! It's awesome that you jumped in and prepared one. I believe that's the best possible approach to bring v2 support fast. IDK whether the v1 compatibility layer of Pydantic is going to last long and whether it has comparable performance as "native" v2, but IMO that's good enough already.

As for the tests, with the change you made, I believe our CI is now only running tests against v2 and is not running tests against v1 anymore. Having tests with both v1 and v2 versions of Pydantic is gonna require quite some more changes to the way the repo is managed.

Regarding the typings issue, I believe we'll need to suppress that one, but not just ignore it. Can you please suppress it with # type: ignore?

Copy link
Member

@xSAVIKx xSAVIKx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@febus982 please let me know if you'd add ignores for MyPy or whether I should jump in and add them.

@febus982
Copy link
Contributor Author

Hey @xSAVIKx

Regarding the typings issue, I believe we'll need to suppress that one, but not just ignore it. Can you please suppress it with # type: ignore?

Yes, I meant ignore for us humans, and suppress it. I'll add # type: ignore

As for the tests, with the change you made, I believe our CI is now only running tests against v2 and is not running tests against v1 anymore. Having tests with both v1 and v2 versions of Pydantic is gonna require quite some more changes to the way the repo is managed.

If this is very important I can try to see if I can configure different tox environments, however as pydantic V1 development is stopped (except for critical bug fixes and security issues) the double time required for tests is something to be considered. Let me know if you want me to address this as well.

@xSAVIKx
Copy link
Member

xSAVIKx commented Aug 28, 2023

Neah, I just checked the internals of the v1 compatibility layer and it feels like they just copied over the whole v1 library 😅 just fixing typings to please mypy and make the check pass is good enough

Signed-off-by: Federico Busetti <729029+febus982@users.noreply.github.com>
@febus982
Copy link
Contributor Author

@xSAVIKx I think it's ready to go (hopefully tests will pass, tox is happy on my local machine)

@xSAVIKx xSAVIKx merged commit 739c71e into cloudevents:main Aug 28, 2023
@febus982 febus982 deleted the pydantic-v2-compatibility branch August 28, 2023 17:44
@febus982 febus982 mentioned this pull request Aug 31, 2023
2 tasks
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.

Support for pydantic v2
3 participants