search

Vonage / vonage-python-sdk

Vonage Server SDK for Python. API support for Voice, SMS, WhatsApp, Verify (2FA), Video Meetings and more.
https://developer.vonage.com
Apache License 2.0
185 stars 112 forks source link

upgrade pydantic #290

Closed dlewis2017 closed 6 months ago

dlewis2017 commented 7 months ago

Upgrade pydantic to 2.5.2 and update syntax (types, function names and input variables)

maxkahan commented 6 months ago

Have also converted this to a draft PR as you suggested was what you wanted in your issue!

dlewis2017 commented 6 months ago

So I added None as the default values to some types which then allowed most tests to pass. Am I over simplifying that or does that make sense? @maxkahan

I ran into an 2 issues though causing 3 tests to fail:

  1. Seems .dict() or .model_dump on a BaseModel of Pydantic no longer serializes nested objects. See Migration Warning above the title and using SerializeAsAny doesn't work for HttpUrl. So right now i get the type error: TypeError: Object of type Url is not JSON serializable

  2. "from" seems to be removed from a few expected json values (connect related). I assume this is a breaking change even though the value in from appears in the base class as well as inside the nested class, so I'll have to figure out why adding the default None value causes this.

I'm still looking into this but if you have any thoughts, please let me know.

maxkahan commented 6 months ago

Hi @dlewis2017 thanks for working on this!

For your comments,:

Re. comment 1. iirc when I was looking at this, I was happy to move from using a specific HttpUrl type to a str type in the short term.

Re. comment 2. The line in the issue you raised can be fixed like this: from_: str = Field(default=None, serialization_alias='from', pattern=r'^[1-9]\d{6,14}$') that worked for me when I was playing with this implementation, you just have to then add the flag by_alias=True to the model_dump field iirc.

Whilst you've been working on this, another contributor has submitted #291 which looks like it's a little further along. Perhaps adding to that PR could be a way to work more efficiently? There are some changes needed that I detailed there.

Thanks again for your consistent enthusiasm on this issue! 🙂

maxkahan commented 6 months ago

Hi @dlewis2017 I received another community PR as mentioned above (#291), which got sorted today so I've just released v3.13.0 which adds support for Pydantic v2. I'll close this PR now, but I appreciate your enthusiasm and please feel free to contribute further in future 🙂

dlewis2017 commented 6 months ago

Perfect, that works, thanks for keeping me updated! @maxkahan