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

Maintain pydantic.v1 model compatibility while using pydantic v2 #393

Merged
merged 7 commits into from
Dec 13, 2024

Conversation

voidnologo
Copy link
Contributor

@voidnologo voidnologo commented Dec 12, 2024

We have pydantic V2 installed in our environemnt, but have been using from pydantic.v1 import .... With the new changes to support pydantic V2 we are wanting to upgrade Spectree to latest and begin writing new serializers, however, we are not ready to update all of our current models to be pydantic v2 BaseModels.

Currently if a ValidationError is raised, the falcon plug in does not catch it, since ValidationError in the plugin is coming from pydantic, not pydantic.v1.

Since you are aliasing the pydantic.v1.ValidationError as InternalValidationError, catching it as well allows us to continue to support our v1 models while we migrate to v2 models.

Looking at the codebase, this looks like a similar issue in the other plugins. Let me know if you would like me to submit this update for them as well. I'm not sure where you would like a test for this in the project.

@voidnologo voidnologo changed the title We have pydantic V2 installed in our environemnt, but have been using from pydantic.v1 import .... With the new changes to support pydantic V2 we are wanting to upgrade Spectree to latest and begin writing new serializers, however, we are not ready to update all of our current models to pydantic be pydantic v2 BaseModels. Maintain pydantic.v1 model compatibility while using pydantic v2 Dec 12, 2024
`from pydantic.v1 import ...`.  With the new changes to support pydantic
V2 we are wanting to upgrade Spectree to latest and begin writing new
serializers, however, we are not ready to update all of our current
models to pydantic be pydantic v2 BaseModels.

Currently if a ValidationError is raised, the falcon plug in does not
catch it, since ValidationError in the plugin is coming from pydantic,
not pydantic.v1.

Since you are aliasing the pydantic.v1.ValidationError as
InternalValidationError, catching it as well allows us to continue
to support our v1 models while we migrate to v2 models.

Looking at the codebase, this looks like a similar issue in the other
plugins.  Let me know if you would like me to submit this update for
them as well.  I'm not sure where you would like a test for this in the
project.
@voidnologo voidnologo force-pushed the pydanticV1_compatability branch from 9eafcd8 to c741462 Compare December 12, 2024 20:14
Copy link

@timschreiner timschreiner left a comment

Choose a reason for hiding this comment

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

I think I ran into this issue last week and it was making me crazy! good find

Copy link
Member

@kemingy kemingy left a comment

Choose a reason for hiding this comment

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

LGTM. It would be great if you could also fix other plugins in the same way.

Adding a test case to verify that v1 validation error can also be caught when using v2 will guarantee that this feature won't be broken in the future.

@kemingy
Copy link
Member

kemingy commented Dec 13, 2024

When I run tests, it is running against the current published version of spectree (1.4.1). I bumped the version to 1.4.2 in pyproject.toml. How do I run tests against the updated code instead?

At least its failing in the way we are seeing in our project.

The test will install the current code:

So it should be the latest commit in this PR.

One thing to note is that you may need to run pytest --snapshot-update if the snapshot test failed due to some changes to the test app API.

@voidnologo
Copy link
Contributor Author

Thanks. That helps. Still trying to figure out the test. Patching the code this way locally in our project fixes the issues; just need to figure out how to write a test here.

Thanks for being so responsive.


@pytest.mark.skipif(not PYDANTIC2, reason="only matters if using both model types")
def test_falcon_validate_both_v1_and_v2_validation_errors(client):
class CompatibilityView:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inlined this view because adding it to the test app then causes issue with the generated spec tests.

Copy link
Member

@kemingy kemingy left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! 🎉

@kemingy kemingy enabled auto-merge (squash) December 13, 2024 16:11
@kemingy kemingy disabled auto-merge December 13, 2024 16:11
@kemingy kemingy merged commit 7160ae5 into 0b01001001:main Dec 13, 2024
7 checks passed
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