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

pydantic: uninformative error #385

Open
zilto opened this issue Oct 3, 2024 · 6 comments · May be fixed by #403
Open

pydantic: uninformative error #385

zilto opened this issue Oct 3, 2024 · 6 comments · May be fixed by #403
Assignees
Labels
good first issue Good for newcomers

Comments

@zilto
Copy link
Collaborator

zilto commented Oct 3, 2024

I'm building an application with burr.integrations.pydantic and building the graph + application fails. The following error message isn't helpful because it exposes internal implementation (i.e., the key "state" is nowhere in my code.

Traceback (most recent call last):
  File "/home/tjean/projects/form_helper/app.py", line 45, in <module>
    @pydantic_action(reads=["form"], writes=["current_field"])
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tjean/projects/form_helper/.venv/lib/python3.11/site-packages/burr/integrations/pydantic.py", line 180, in decorator
    itype, otype = _validate_and_extract_signature_types(fn)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tjean/projects/form_helper/.venv/lib/python3.11/site-packages/burr/integrations/pydantic.py", line 116, in _validate_and_extract_signature_types
    if (state_model := type_hints["state"]) is inspect.Parameter.empty or not issubclass(
                       ~~~~~~~~~~^^^^^^^^^
KeyError: 'state'

The only hint is to look at "line 45", but I can't tell what's the issue. I ended up finding that the issue is the function signature on line 46.

Original:

@pydantic_action(reads=["form"], writes=["current_field"])
def prompt_user(state):

Fix:

@pydantic_action(reads=["form"], writes=["current_field"])
def prompt_user(state: BurrState):
@elijahbenizzy elijahbenizzy added the good first issue Good for newcomers label Oct 3, 2024
@elijahbenizzy
Copy link
Contributor

This should be as simple as throwing the appropriate error message here

@rajatkriplani
Copy link

Hi @elijahbenizzy can't I just simply add:

state_model = type_hints.get("state", None)

in line 115 of pydantic.py and then change the if condition to raise ValueError

@elijahbenizzy
Copy link
Contributor

here

Yep, I think that will work. In fact, you don't have to have the None there, you can just do .get(...) which returns None if it's not there!

@rajatkriplani
Copy link

rajatkriplani commented Oct 16, 2024

Yep, I think that will work. In fact, you don't have to have the None there, you can just do .get(...) which returns None if it's not there!

Oh yes, thanks for reminding me that. Please let me know if if statement is checking all the conditions correctly:

state_model = type_hints.get("state")

    if state_model is None or state_model is inspect.Parameter.empty or not issubclass(state_model, pydantic.BaseModel):
        raise ValueError(
            f"Function fn: {fn.__qualname__} is not a valid pydantic action. "
            "The 'state' parameter must be annotated with a type extending pydantic.BaseModel."
        )

@rajatkriplani
Copy link

Hey @elijahbenizzy @zilto can anyone of you give me feedback on the if condition?

@elijahbenizzy
Copy link
Contributor

Hey @elijahbenizzy @zilto can anyone of you give me feedback on the if condition?

Hey! Looks good at first glance. Open a PR and add some tests, then I can give you more feedback there!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
3 participants