-
Notifications
You must be signed in to change notification settings - Fork 172
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
Support both v1 and v2 of Pydantic #164
Conversation
One of the nice parts about pydantic v2 is that they provide backward compability by importing from `pydantic.v1`. This adds support so that regardless of which version of pydantic the project is using, py_webauthn will work.
I do see there's a branch to upgrade to Pydantic V2, this feels like a nice stepping stone for people using the library where both will work. |
@@ -4,7 +4,7 @@ | |||
try: | |||
from pydantic.v1 import BaseModel | |||
except ImportError: | |||
from pydantic import BaseModel | |||
from pydantic import BaseModel # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the #type: ignore
's on the v2 imports because mypy, referencing v1 types, would complain about these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The imports in the except
block are actually "v1" imports.
If we can't import from pydantic.v1
(which only exists in v2), then we instead try to import from pydantic
(assuming that v1 is installed).
Because in the repo itself we have v2 installed, I've put the ignores on the "v1" imports because when v2 is installed that's actually importing the new v2 API which conflicts with the old v1 version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right pydantic.v1
is the new import 🤦♂️
So would this mean users still using Pydantic v1 in their project wouldn't get mypy-assisted type annotations on the methods in this library if they're still on Pydantic v1? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah, I think that's probably right (I have no idea how you'd go about supporting that either) 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, depending on how much of the internal pydantic stuff is being exposed, I suspect it probably won't be that much of a degraded experience for those folks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So would this mean users still using Pydantic v1 in their project wouldn't get mypy-assisted type annotations on the methods in this library if they're still on Pydantic v1? 🤔
From what I've seen mypy will also complain about a lot of stuff if you have v2 installed and use the v1 imports, at least if the mypy plugin for pydantic is enabled.
This seems like a nice stepping stone for unblocking downstream projects from upgrading to Pydantic v2, but in my experience using the v1 imports is a bit clumsy in a project where you mainly use v2 features. For example when you have the And if this is the approach we go for it should probably be well documented, as any subclasses of |
I've attempted an alternative approach in #166, turns out Pydantic v2 has native support for serializing bytes to base64 encoded string, and it appears to be using the URL safe variant, so that might be a viable path |
@MasterKale - Are you just going to do the split-version thing (i.e. not use this MR) or still undecided? I'm just sort of hanging out to upgrade to pydantic2 in my own project and trying to figure out how I'm going to proceed (looking for some idea of a timeline on this if you have any idea what that might be)? |
PR #166 caught my attention and I thought that was going to end up being the way to support v1 and v2, but after coming back to this based on your nudge I think this will probably be the intermediate step. Then I can spend some time working on updating to v2 (though I'm hoping pydantic/pydantic#7000 gets some support because it'll make the migration a lot smoother.) I'll take a look again in the morning, it's till Sunday evening for me. The fact that tests are passing in this branch gives me hope that I can quickly merge and release as v1.10.0 tomorrow, or Tuesday at the latest to give me some wiggle room. |
Awesome, thanks heaps for that 👍
…On Mon, 7 Aug 2023 at 4:16 PM, Matthew Miller ***@***.***> wrote:
PR #166 <#166> caught my
attention and I thought that was going to end up being the way to support
v1 and v2, but after coming back to this based on your nudge I think this
will probably be the intermediate step. Then I can spend some time working
on updating to v2 (though I'm hoping pydantic/pydantic#7000
<pydantic/pydantic#7000> gets some support
because it'll make the migration a lot smoother.)
I'll take a look again in the morning, it's till Sunday evening for me.
The fact that tests are passing in this branch gives me hope that I can
quickly merge and release as v1.10.0 tomorrow, or Tuesday at the latest to
give me some wiggle room.
—
Reply to this email directly, view it on GitHub
<#164 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACDQFPXFETWEGTPRX6BNDDXUBT2VANCNFSM6AAAAAA2656M6I>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Hi @MasterKale , any luck having a look at making a v1.10.0? |
@martynsmith Thank you for creating this PR. I've opted for PR #166 to add intermediate Pydantic V2 support so I'm going to close this one out. |
One of the nice parts about pydantic v2 is that they provide backward compability by importing from
pydantic.v1
. This adds support so that regardless of which version of pydantic the project is using, py_webauthn will work.