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

Add backward compatibility with Pydantic V1 #92

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Merinorus
Copy link
Contributor

@Merinorus Merinorus commented Jun 14, 2024

Add back the support of Pydantic V1 (Implements #90). This eases the migration from Pydantic V1 to Pydantic V2.

Without this support, users cannot gradually migrate a Flask project from Pydantic V1 to Pydantic V2.
I tried to keep the code change to a minimum. However, test files are in a separate folder, to ease the migration when Pydantic V1 support would be completely dropped.

No noticeable performance impact for Pydantic V2 users.

@Merinorus Merinorus force-pushed the issue-90-compatibility-with-pydantic-v1-basemodel branch from 738cfe9 to 5b5089d Compare June 14, 2024 11:22
@Merinorus Merinorus changed the title Keep retrocompatibility with Pydantic V1 base models Keep backward compatibility with Pydantic V1 base models Jun 14, 2024
@Merinorus Merinorus force-pushed the issue-90-compatibility-with-pydantic-v1-basemodel branch 3 times, most recently from f1e0d31 to 2b99f09 Compare June 14, 2024 16:12
@Merinorus Merinorus changed the title Keep backward compatibility with Pydantic V1 base models Keep backward compatibility with Pydantic V1 Jun 14, 2024
@Merinorus Merinorus force-pushed the issue-90-compatibility-with-pydantic-v1-basemodel branch 2 times, most recently from 35f7c96 to 0dc676f Compare June 16, 2024 19:01
Copy link

sonarcloud bot commented Jun 16, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Merinorus Merinorus changed the title Keep backward compatibility with Pydantic V1 Add backward compatibility with Pydantic V1 Jun 16, 2024
@yctomwang
Copy link
Collaborator

@Merinorus hey, thanks for the PR, it seems like this PR some tests on the CI. I will have a closer looked at which tests are failing and why they are failing, but I think this PR could really benefit if we support both V1 and V2!

@Merinorus Merinorus force-pushed the issue-90-compatibility-with-pydantic-v1-basemodel branch 4 times, most recently from be25454 to da0a048 Compare June 18, 2024 15:59
@Merinorus
Copy link
Contributor Author

Merinorus commented Jun 18, 2024

Hi @yctomwang, thank you! I made a little mistake on instance checking:

isinstance(obj, Union[BaseModel, V1BaseModel])  # Wrong
->
isinstance(obj, (BaseModel, V1BaseModel))  # Ok

This should be good for review now.
Tests fail on the macOS runners, but it seems unrelated to the PR.

@yctomwang
Copy link
Collaborator

@Merinorus Hey, I dont think the MacOS failures are because of your PR, give me sometime to investigate into why the CI is failing and i will get back to you. Thank you once again for the PR and your patience!

@Merinorus Merinorus mentioned this pull request Jun 24, 2024
@Merinorus
Copy link
Contributor Author

@yctomwang Hi! I found out why the CI is failing: the macOS workers are now running ARM architecture, and python 3.7 is not supported anymore. So if you want to continue supporting this version, this might need to be tested against an old macOS version running on x86 architecture. The setup-python action needed to be upgraded as well.
You can check this PR: #94

@Merinorus Merinorus mentioned this pull request Jun 24, 2024
@yctomwang
Copy link
Collaborator

@Merinorus with the CI fixed and we can merge code, i think we can proceed with this PR. for somereason this PR right now still fails actions even tho its been rerun.

@Merinorus
Copy link
Contributor Author

Hi @yctomwang, no worries. It's because this PR should be rebased on the latest commit from the main branch, otherwise it runs on the old CI. I'll rebase the branch.

@Merinorus Merinorus force-pushed the issue-90-compatibility-with-pydantic-v1-basemodel branch from da0a048 to 4bff748 Compare July 13, 2024 18:08
@Merinorus Merinorus force-pushed the issue-90-compatibility-with-pydantic-v1-basemodel branch from 4bff748 to 5c7dad9 Compare July 13, 2024 18:22
@Merinorus
Copy link
Contributor Author

All good :)

@goodmattg
Copy link

Hi there - what's the timeline to merge and release this PR? This is blocking our migration as well

@Merinorus
Copy link
Contributor Author

Merinorus commented Oct 4, 2024

I would be happy to proceed but I'm not a maintainer. In the meantime, you can fix your application requirements with the PR's commit.

Example of a python requirement.txt:

flask-Pydantic @ git+https://github.com/merinorus/flask-pydantic.git@da0a0489474d2b01c61a1f51d5b7536647e2ad29
...
pydantic==2.9.2

Fork it to your personal or organization repo, so you won't depend on mine.

@stv8
Copy link

stv8 commented Oct 30, 2024

+1 any update on getting this merged into core?

We have forked it in the mean time, but this is extremely helpful as an upgrade path from v1 to v2 and it would be nice to get it in

@Merinorus
Copy link
Contributor Author

Hi @yctomwang, any news? I didn't find a way to contact you personally.
If you need some people to collaborate, I'd happily join!

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.

4 participants