BeanieODM / beanie

Asynchronous Python ODM for MongoDB
http://beanie-odm.dev/
Apache License 2.0
1.99k stars 211 forks source link

fix: issue#728 pydantic_core._pydantic_core.Url object is not iterable #730

Closed tomohirohiratsuka closed 11 months ago

tomohirohiratsuka commented 11 months ago

Hi, To fix #728 which causing below errors, I've added Url type in encoder. I'd appreciate if you check it. Thank you.

ValueError: [TypeError("'pydantic_core._pydantic_core.Url' object is not iterable"), TypeError('vars() argument must have __dict__ attribute')]
gsakkis commented 11 months ago

@roman-right before fixing this please merge https://github.com/roman-right/beanie/pull/584. It's been open for 4 months and I already rebased and fixed the conflicts once, I probably won't bother again.

roman-right commented 11 months ago

Hi @tomohirohiratsuka Sorry, I've produced a merge conflict here. Please let me know, if you can resolve it, otherwise I'll pick it up.

tomohirohiratsuka commented 11 months ago

@roman-right Hi, I've resolved conflicts, please check it. Thank you.

roman-right commented 11 months ago

Hey @tomohirohiratsuka , It looks like a few checks failed. Could you please take a look?

tomohirohiratsuka commented 11 months ago

@roman-right Hi, I've fixed the test case though it is a not perfectly clean since it has if condition inside the test. Since pydantic v1, v2 url transform logic is changed like below. I've confirmed beanie can handle both v1 same as before, v2 to fix bug.

Could you check changes?

from pydantic import BaseModel, HttpUrl

class HttpUrlModel(BaseModel):
    url: HttpUrl

pydantic v1

from main import HttpUrlModel

def test_debug():

    model = HttpUrlModel(url="https://www.google.com")
    assert model.url == "https://www.google.com" # True
    assert isinstance(model.url, str) # True

pydantic v2

from pydantic_core import Url

from main import HttpUrlModel

def test_debug():

    model = HttpUrlModel(url="https://www.google.com")
    # assert model.url == "https://www.google.com" # This is not true in v2
    # assert isinstance(model.url, str) # This is not true in v2
    assert isinstance(model.url, Url) # This is true in v2, Url object is a new object in V2.
roman-right commented 11 months ago

@tomohirohiratsuka Having different tests for different Pydantic versions is ok. I left a few comments, please take a look

roman-right commented 11 months ago

Merged! Thank you for the PR. It will be published tomorrow.