litestar-org/polyfactory

Bug: pydantic HttpUrl type causes ValidationError after 2.3.x

comfuture opened this issue ยท 5 comments

Description

After version 2.3.0, it fails to validate schemas correctly, such as pytantic.networks.HttpUrl in pydantic_factory, and throws an exception.

E   pydantic.error_wrappers.ValidationError: 1 validation error for `MyModel`
E   url
E     invalid or missing URL scheme (type=value_error.url.scheme)

URL to code causing the issue

No response

MCVE

from typing import Optional
from pydantic import BaseModel, HttpUrl
from polyfactory.factories.pydantic_factory import ModelFactory

class MyModel(BaseModel):
    url: Optional[HttpUrl] = Field(None, title='An image URL')

# ...
class TestFactory(ModelFactory[MyModel]):
    __model__ = MyModel

my_factory = TestFactory.build()
my_instance = my_factory() # <--

E pydantic.error_wrappers.ValidationError: 1 validation error for MyModel
E url
E invalid or missing URL scheme (type=value_error.url.scheme)

Steps to reproduce

1. Prepare pydantic model that has `HttpUrl` type field.
2. Prepare factory using pydantic_factory.ModelFactory
3. Try to create instance using factory(2)
4. See error

Screenshots

"In the format of: ![SCREENSHOT_DESCRIPTION](SCREENSHOT_LINK.png)"

Logs

No response

Litestar Version

2.3.0

Platform

  • Linux
  • Mac
  • Windows
  • Other (Please specify in the description above)
Fund with Polar

Hi, yes we had (have?) a regression. Have you tested v2.3.2?

Aloha! I do have polyfactory==2.3.2 set, but still getting an issue.

After a little investigation I found out that this 'if' block was the source of the ruckus. So I added unwrap_optional(annotation) and it worked like a charm

P. S. workaround for existing codebase

class PatchedPydanticFieldMeta(PydanticFieldMeta):
    @classmethod
    def from_model_field(
        cls,
        model_field: ModelField,
        use_alias: bool,
    ) -> PydanticFieldMeta:  # noqa
        from pydantic import AmqpDsn, AnyHttpUrl, AnyUrl, HttpUrl, KafkaDsn, PostgresDsn, RedisDsn
        pfm = super().from_model_field(model_field, use_alias)
        if unwrap_optional(pfm.annotation) in (AmqpDsn, AnyHttpUrl, AnyUrl, HttpUrl, KafkaDsn, PostgresDsn, RedisDsn):
            pfm.constraints = None
        return pfm


class PatchedModelFactory(Generic[T], ModelFactory[T]):
    __is_base_factory__ = True

    @classmethod
    def get_model_fields(cls) -> list["FieldMeta"]:
        # Only works for pydantic v1.x
        cls._fields_metadata = [
            PatchedPydanticFieldMeta.from_model_field(
                field,
                use_alias=not cls.__model__.__config__.allow_population_by_field_name,
            )
            for field in cls.__model__.__fields__.values()  # type: ignore[attr-defined]
        ]
        return cls._fields_metadata

Aloha! I do have polyfactory==2.3.2 set, but still getting an issue.

After a little investigation I found out that this 'if' block was the source of the ruckus. So I added unwrap_optional(annotation) and it worked like a charm

P. S. workaround for existing codebase

class PatchedPydanticFieldMeta(PydanticFieldMeta):
    @classmethod
    def from_model_field(
        cls,
        model_field: ModelField,
        use_alias: bool,
    ) -> PydanticFieldMeta:  # noqa
        from pydantic import AmqpDsn, AnyHttpUrl, AnyUrl, HttpUrl, KafkaDsn, PostgresDsn, RedisDsn
        pfm = super().from_model_field(model_field, use_alias)
        if unwrap_optional(pfm.annotation) in (AmqpDsn, AnyHttpUrl, AnyUrl, HttpUrl, KafkaDsn, PostgresDsn, RedisDsn):
            pfm.constraints = None
        return pfm


class PatchedModelFactory(Generic[T], ModelFactory[T]):
    __is_base_factory__ = True

    @classmethod
    def get_model_fields(cls) -> list["FieldMeta"]:
        # Only works for pydantic v1.x
        cls._fields_metadata = [
            PatchedPydanticFieldMeta.from_model_field(
                field,
                use_alias=not cls.__model__.__config__.allow_population_by_field_name,
            )
            for field in cls.__model__.__fields__.values()  # type: ignore[attr-defined]
        ]
        return cls._fields_metadata

Cool. PR is welcome

It feels like trying to ensure interop w/ PydanticV2 is a little bit ... precarious? Saw some really 'sketchy' hacks within the codebase. Yet, not in the right position to judge ๐Ÿ˜ƒ

Broke the regress w/ a super simple change and fixed w/ peculiar patch (yet it works and behaves as expected). Reviews are welcome as well ๐Ÿ‘

It feels like trying to ensure interop w/ PydanticV2 is a little bit ... precarious? Saw some really 'sketchy' hacks within the codebase. Yet, not in the right position to judge ๐Ÿ˜ƒ

Broke the regress w/ a super simple change and fixed w/ peculiar patch (yet it works and behaves as expected). Reviews are welcome as well ๐Ÿ‘

Yes i don't like it at all. But we didbt get a choice in the matter ๐Ÿค”