Upgrade Pydantic Dependency to Pydantic 2 #217
Conversation
esciara
force-pushed
the
qmalcolm--pydantic-2-support
branch
from
8806001 to
4b6bf94
Compare
pyproject.toml
Outdated
| @@ -20,7 +20,7 @@ classifiers = [ | |||
| "Programming Language :: Python :: Implementation :: PyPy", | |||
| ] | |||
| dependencies = [ | |||
| "pydantic~=1.10", | |||
| "pydantic~=2.5", | |||
There was a problem hiding this comment.
| "pydantic~=2.5", | |
| "pydantic>=2.5", |
You'll probably want to wait for someone from DBT to approve but it would be great to have a looser constraint here and adding an upper bound when/if it becomes necessary.
There was a problem hiding this comment.
Hi. Yes I will let them say what to do, as I only followed how it was done before.
There was a problem hiding this comment.
Howdy! I actually think we should stick with the ~= specification. Pydantic follows semantic versioning pretty well, at least I believe it does. As such we should expect a Pydantic 3 to be breaking. It's also generally less painful after the fact to loosen the top end as compared to having to tighten it.
I have a separate question though, is there a specific reason to use ~=2.5 instead of ~=2.0 here? Are we using any 2.5 specific functionality in this change that requires 2.5 to be the minimum?
There was a problem hiding this comment.
No specific reason. I probably should not have done that.
I will change it tomorrow and do some tests on lower versions than 2.5, just to make sure nothing breaks.
There was a problem hiding this comment.
I confirm that tests pass even with pydantic ==2.0.0, so no need for ~=2.5. I put it back to ~=2.0.
@QMalcolm the floor is yours.
|
It looks like this has drifted and may need rebasing @esciara, are you able to do that? Still needs someone from dbt to review unfortunately, they're busy folks |
Yes will do (before the end of the weekend hopefully). |
|
Awesome! I was hesitant to ask. This is my last hard blocker on a pydantic 2 migration unfortunately, as I imagine it is for others. Thankyou so much for working on it! |
|
Came to add huge thanks to @esciara for the legwork and add a +1 to the PR, hit this dependency conflict when trying to upgrade |
In pydantic 1.x `Optional` fields on pydantic objects automatically got defaulted to `None`. That is no longer the case in pydantic 2.x.
esciara
force-pushed
the
qmalcolm--pydantic-2-support
branch
from
28a9226 to
547ff92
Compare
|
You are welcome @bernard-arcturisdata . 🤗 @robcresswell => done. Would be great if this could be merged before needing rebasing again: it was kinda hard to get back into it, and the last fixing commit I did gave me a bit of a hard time. |
esciara
force-pushed
the
qmalcolm--pydantic-2-support
branch
from
0e3e60d to
aeeadc2
Compare
|
I don't know much It will help but https://getdbt.slack.com/archives/C50NEBJGG/p1699935532376859 is a relevant slack thread in DBT's core-dev channel. May be worth more activity in there to help encourage DBT to look at it. |
QMalcolm
changed the title
|
As we've been reviewing this we also realized we are likely going to have to support both Pydantic 1 and Pydantic 2 for quite some time. About 60 percent of people using Pydantic are still on Pydantic 1. Pydantic 2 is very exciting. It's faster, and we plan to support it in 0.5 of DSI. Although we'd love to cut directly to just supporting Pydantic 2, that'll likely cause immediate pain for community. In regards to this PR, we still want to get it in. Currently we're investigating whether the changes in this PR will also work with any Pydantic 1 versions (hopefully at least 1.10.x). If that is not the case, we're then going to investigate if that would be possible with some slight tweaks to this PR. If that does not work, there are some crunchier ways for us to support both which we'd likely do in a separate PR if it came to that. |
|
@esciara I also want to thank you again for this work. This is a very well put together PR. Regardless of what direction we end up having to take, this work has forced us to dive deeper into this issue sooner than we likely would have otherwise. |
|
@QMalcolm your are welcome. 🤗 |
|
By the way, it seems to me that fastapi supports both pydantic v1 and v2. I have not clue how they do this, but it might be interesting to have a look. |
At the risk of over stepping my bounds. Isn't the immediate pain they'd experience limited to only the inability to update to any version of DBT that requires the new DSI until they update their dependencies? Whereas currently the pain for those of us in the community who have updated to pydantic>2 is that we are restricted to the versions of DBT <=1.5 (which do not conflict with pydantic >2) ... I agree that supporting both would be the smoothest for the community but that might also introduce a significant maintenance burden, slowing development on this library. Anyways just wanted to make sure you considered all that. Nevertheless thank you both for working on this. As you might have guessed it's become a significant pain point for me 😁 |
|
In the spirit of "trying to help", I raised a PR #227 with what I'd call an interim solution - it's not as nice as being able to "just upgrade" but I think it would go a long way towards mitigating the impact of having the dependency pinned 🙂 |
|
@esciara I think for the time being we're going to close this PR. We want to include this PR in the future, but that future is likely 1+ year(s) away. At that time we'll look to re-open this PR 🙂 |
Resolves #134
Description
All comments are already on the issue.
Probably need a bit more cleaning up.
It is to be noted that there is a change of behavior by
pydanticwhere by casting is no longer done on attributes. Not knowing which behavior was desired, I changed the tests rather that the code, making sure I wrote a# TODOto make it clearly visible.Checklist
changie newto create a changelog entry