search

bauerji / flask-pydantic

flask extension for integration with the awesome pydantic package
MIT License
352 stars 56 forks source link

Add backward compatibility with Pydantic V1 #92

Open Merinorus opened 2 months ago

Merinorus commented 2 months ago

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.

sonarcloud[bot] commented 2 months ago

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

yctomwang commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

@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: https://github.com/bauerji/flask-pydantic/pull/94

yctomwang commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

All good :)

goodmattg commented 1 month ago

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