ioxiocom / firedantic

Database models for Firestore using Pydantic base models.
BSD 3-Clause "New" or "Revised" License
43 stars 14 forks source link

Update pydantic to v2.x #45

Closed omarudolley closed 10 months ago

omarudolley commented 10 months ago
antont commented 10 months ago

Just for info, I ported our app to this branch, and all seems to be working fine.

Earlier I had sync code with Pydantic v1, using my own simple db util to get validated pydantic objs nicely from Firestore documents. But switched it now so that everything is using Firedantic and this Pydantic v2 version, in Async fashion.

Tests pass and using it locally seems to work, we'll review the change now a bit and I'll probably deploy it to actual testing on the cloud in the afternoon, so will see more.

Thanks a lot for the work here! Is nice, even though there are things to do too.

+1 for calling this 0.5.0

joakimnordling commented 10 months ago

Just for info, I ported our app to this branch, and all seems to be working fine.

Earlier I had sync code with Pydantic v1, using my own simple db util to get validated pydantic objs nicely from Firestore documents. But switched it now so that everything is using Firedantic and this Pydantic v2 version, in Async fashion.

Tests pass and using it locally seems to work, we'll review the change now a bit and I'll probably deploy it to actual testing on the cloud in the afternoon, so will see more.

Thanks a lot for the work here! Is nice, even though there are things to do too.

+1 for calling this 0.5.0

Thanks for the feedback and for testing it out already!

lietu commented 10 months ago

It seems this will at a minimum cause warnings as-is because it wasn't fully converted to use Pydantic >= 2 format, and e.g. the branch seems to still contain calls to .dict() instead of .model_dump()

https://github.com/omarudolley/firedantic/blob/7761f6bcd36be5dd507635819201802624101798/firedantic/_async/model.py#L71

https://github.com/omarudolley/firedantic/blob/7761f6bcd36be5dd507635819201802624101798/firedantic/_async/model.py#L282

antont commented 10 months ago

It seems this will at a minimum cause warnings as-is because it wasn't fully converted to use Pydantic >= 2 format, and e.g. the branch seems to still contain calls to .dict() instead of .model_dump()

Yep those give warnings, I was considering porting in my fork but figured to wait for them to get converted here maybe, is not urgent.

lietu commented 10 months ago

It seems this will at a minimum cause warnings as-is because it wasn't fully converted to use Pydantic >= 2 format, and e.g. the branch seems to still contain calls to .dict() instead of .model_dump() https://github.com/omarudolley/firedantic/blob/7761f6bcd36be5dd507635819201802624101798/firedantic/_async/model.py#L282

https://github.com/omarudolley/firedantic/blob/feature/update_pydantic_to_v2.x/firedantic/_async/model.py#L282

lietu commented 10 months ago

Notify https://github.com/ioxiocom/firedantic/issues/42#issuecomment-1751916733 once merged