Closed esciara closed 10 months ago
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
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 dbt-core
. Without it we'll be stuck on v1.5.9 for a while!
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.
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.
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.
Although we'd love to cut directly to just supporting Pydantic 2, that'll likely cause immediate pain for community.
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
pydantic
where 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# TODO
to make it clearly visible.Checklist
changie new
to create a changelog entry