Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect exception raised when nested model validation fails #449

Closed
1 of 4 tasks
alessio-locatelli opened this issue Dec 7, 2023 · 3 comments · Fixed by #450
Closed
1 of 4 tasks

incorrect exception raised when nested model validation fails #449

alessio-locatelli opened this issue Dec 7, 2023 · 3 comments · Fixed by #450
Labels
bug Something isn't working

Comments

@alessio-locatelli
Copy link

alessio-locatelli commented Dec 7, 2023

Description

Problem 1

Can not handle Predicate in a model that is used in a field of a primary model.

Problem 2

Misleading, wrong error.

The error says polyfactory.exceptions.ParameterException: received constraints for unsupported type list[__main__.Pet] but the real error is much deeper, in a specific field:

    codes: list[Code]

MCVE

from typing import Annotated
from annotated_types import MinLen, Predicate
from pydantic import BaseModel
from polyfactory.factories.pydantic_factory import ModelFactory


IsUpper = Predicate(str.isupper)
Code = Annotated[str, IsUpper]


class Pet(BaseModel):
    codes: list[Code]


class Person(BaseModel):
    pets_names: Annotated[list[Pet], MinLen(1)]


class PersonFactory(ModelFactory[Person]):
    __model__ = Person


print(PersonFactory.build())

Logs

Traceback (most recent call last):
  File "/opt/pysetup/try_polyfactory.py", line 23, in <module>
    print(PersonFactory.build())
          ^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pysetup/.venv/lib/python3.11/site-packages/polyfactory/factories/pydantic_factory.py", line 377, in build
    processed_kwargs = cls.process_kwargs(**kwargs)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pysetup/.venv/lib/python3.11/site-packages/polyfactory/factories/base.py", line 844, in process_kwargs
    result[field_meta.name] = cls.get_field_value(field_meta, field_build_parameters=field_build_parameters)
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pysetup/.venv/lib/python3.11/site-packages/polyfactory/factories/base.py", line 617, in get_field_value
    return cls.get_constrained_field_value(annotation=unwrapped_annotation, field_meta=field_meta)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pysetup/.venv/lib/python3.11/site-packages/polyfactory/factories/pydantic_factory.py", line 364, in get_constrained_field_value
    return super().get_constrained_field_value(annotation, field_meta)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/pysetup/.venv/lib/python3.11/site-packages/polyfactory/factories/base.py", line 586, in get_constrained_field_value
    raise ParameterException(msg)
polyfactory.exceptions.ParameterException: received constraints for unsupported type list[__main__.Pet]

Expected behavior

Ignore Annotated.__metadata__ in a model that is used in a field and continue execution.

Release Version

Version: 2.12.0

Platform

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

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
@alessio-locatelli alessio-locatelli added the bug Something isn't working label Dec 7, 2023
@alessio-locatelli alessio-locatelli changed the title polyfactory fails because can not handle Annotated.__metadata__ when it is in a model used in a field polyfactory fails because can not handle Annotated.__metadata__ when it is in a model used in a field of another model Dec 7, 2023
@guacs
Copy link
Member

guacs commented Dec 8, 2023

@olk-m thanks for reporting this. The Annotated.__metadata__ is already being parsed in order to generate values that satisfy the constraints. So, it seems there's some bug where it's not getting parsed or something. Also, the error message is indeed unexpected.

@guacs guacs changed the title polyfactory fails because can not handle Annotated.__metadata__ when it is in a model used in a field of another model incorrect exception raised when nested model validation fails Dec 8, 2023
@guacs
Copy link
Member

guacs commented Dec 8, 2023

The reason this was failing is because polyfactory will sometimes create an empty string which fails the validation call to str.isupper. The exception thrown here was being supressed by polyfactory instead of properly allowing it to be raised. This has been fixed in #450.

This now poses a question on how to handle the cases where isupper or islower constraints are provided, but no min_length constraint is given. The options are:

  • throw an exception if min_length is not provided
  • default to a minimum length of 1 if min_length is not provided

Personally, I'm leaning towards the first option. It's more explicit whereas the second option is implicit.

@alessio-locatelli
Copy link
Author

alessio-locatelli commented Dec 8, 2023

Thank you for a quick response.

Personally, I'm leaning towards the first option.

I agree that if a user uses a bare str or Annotated[str, IsUpper] (without MinLen(1)) then polyfactory should not silently default to a non-empty string.

@guacs guacs closed this as completed in #450 Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants