dbt-labs / dbt-semantic-interfaces

The shared semantic layer definitions that dbt-core and MetricFlow use.
Apache License 2.0
71 stars 14 forks source link

Begin supporting Pydantic 2 (without dropping support for Pydantic 1) #238

Closed QMalcolm closed 10 months ago

QMalcolm commented 10 months ago

Resolves #134

Description

After community and internal discussions we decided the best path forward was to support both Pydantic 1 and Pydantic 2 for the time being as we can't drop Pydantic 1 yet, but we also need to allow for Pydantic 2. We decided the tidiest way to do this would be via shimming. This PR adds the shim, directs pydantic imports to it, loosens the pydantic requirement to allow for pydantic 2, and ensure our testing checks both moving forward.

Also, I want shout out other PRs that worked to address this issue #217 and #227 as well as the people who worked on them (@esciara and @bernardcooke53 respectively). Their work was fundamental to making the changes that this PR contains, and they should be considered contributors on this effort.

Checklist

QMalcolm commented 10 months ago

@esciara & @bernardcooke53 can you both confirm you are okay with me including y'all as authors in the changie log? ❤️

bernardcooke53 commented 10 months ago

@QMalcolm absolutely! 🚀

QMalcolm commented 10 months ago

Also because we aren't dropping support for Pydantic 1 in this work, I think we can backport this to 0.4.latest. Which would then allow for people using dbt-core and Pydantic 2 to upgrade dbt-core 1.7 and not have to continue waiting until dbt-core 1.8. But we should do some more testing first.

esciara commented 10 months ago

@esciara & @bernardcooke53 can you both confirm you are okay with me including y'all as authors in the changie log? ❤️

Sure is!

esciara commented 10 months ago

I can see that you ensure compatibility by keeping the code as is and importing the package pydantic.v1 in case Pydantic v2 is selected. Probably the smartest choice! 👍

So I guess that when you drop support for Pydantic v1, my PR #217 might become useful. I leave you do decide how to handle it (close it, keep it open, add a comment to it, other...).

RoelantDL commented 10 months ago

Great improvement! Does anyone have an estimate on when this pull request will be merged?

QMalcolm commented 10 months ago

@RoelantDL With review approval this morning, it'll be merged today. It's worth noting that merging it won't make it immediately available in dbt-core. Core 1.7.x runs on dbt-semantic-interfaces 0.4.x, thus we need to backport this to 0.4.latest and then release an 0.4.3 of dbt-semantic-interfaces. At that point, you'll have refresh your environment to pull in the new version of dbt-semantic-interfaces.