litestar-org / polyfactory

Simple and powerful factories for mock data generation
https://polyfactory.litestar.dev/
MIT License
988 stars 78 forks source link

Bug: UUIDBase ID Value Changed from UUID to Bytes #511

Closed junhopark97 closed 5 months ago

junhopark97 commented 5 months ago

Description

In the example below, UserFactory.build().id outputs a UUID.

When upgrading from version polyfactory==2.14.1 to polyfactory==2.15.0, it is observed that UserFactory.build().id now outputs Bytes instead of a UUID.

Is the output in Bytes intended? Am I missing something?

MCVE

from advanced_alchemy.base import UUIDBase
from polyfactory.factories.sqlalchemy_factory import SQLAlchemyFactory

class User(UUIDBase):
    pass

class UserFactory(SQLAlchemyFactory[User]):
    pass

print("id: ", UserFactory.build().id)

Steps to reproduce

No response

Screenshots

No response

Logs

# polyfactory==2.14.1
id: 8cb1dff7-5232-48ba-a249-99946e4c13af

# polyfactory==2.15.0
id: b'58'

Release Version

2.15.0

Platform


[!NOTE]
While we are open for sponsoring on GitHub Sponsors and OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.

Fund with Polar

guacs commented 5 months ago

Thanks for reporting this @junhopark97! I am able to reproduce this. According to git bisect, it seems #502 broke something.

guacs commented 5 months ago

Actually, this may be due to something in advanced-alchemy because the following works:

from uuid import UUID

from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column

from polyfactory.factories.sqlalchemy_factory import SQLAlchemyFactory

class Base(DeclarativeBase):
    ...

class User(Base):
    __tablename__ = "users"

    id: Mapped[UUID] = mapped_column(primary_key=True)

class UserFactory(SQLAlchemyFactory[User]):
    pass

assert isinstance(UserFactory.build().id, UUID)

@cofin any ideas?

adhtruong commented 5 months ago

Looking here https://github.com/jolt-org/advanced-alchemy/blob/main/advanced_alchemy/types/guid.py#L38 it seems the mapped typed for UUID in advanced alchemy uses binary under the hood.

By my understanding, TypeEngine.impl.python_type would be the internal type passed to a particular dialect engine. I think should prioritise TypeEngine.python_type over TypeEngine.impl.python_type

junhopark97 commented 5 months ago

Thanks @guacs, @adhtruong!