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 V2 - Error when parsing a pull_request webhook event #50

Closed
frankie567 opened this issue Oct 2, 2023 · 2 comments · Fixed by #57
Closed

Pydantic V2 - Error when parsing a pull_request webhook event #50

frankie567 opened this issue Oct 2, 2023 · 2 comments · Fixed by #57
Labels
bug Something isn't working WebHook

Comments

@frankie567
Copy link
Contributor

frankie567 commented Oct 2, 2023

I'm currently trying the latest alpha with Pydantic V2 and I encounter an error when trying to parse a pull_request webhook event.

Here is a reproducible example:

from githubkit.webhooks import parse_obj

payload = {
    "action": "opened",
    "assignee": None,
    "number": 1,
    "pull_request": {},
    "repository": {},
    "sender": {},
}

event = parse_obj("pull_request", payload)

Expected behavior

The payload should be parsed.

Actual behavior

Pydantic raises a TypeError, complaining that the discrimininator action is mapped to multiple choices.

Stack trace
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/githubkit/webhooks/parse.py", line 27, in parse_obj
    return TypeAdapter(webhook_event_types[name]).validate_python(payload)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/type_adapter.py", line 243, in __init__
    core_schema = _discriminated_union.apply_discriminators(_core_utils.simplify_schema_references(core_schema))
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_discriminated_union.py", line 57, in apply_discriminators
    return simplify_schema_references(_core_utils.walk_core_schema(schema, inner))
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_core_utils.py", line 426, in walk_core_schema
    return f(schema.copy(), _dispatch)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_discriminated_union.py", line 45, in inner
    s = recurse(s, inner)
        ^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_core_utils.py", line 202, in walk
    return f(schema, self._walk)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_discriminated_union.py", line 45, in inner
    s = recurse(s, inner)
        ^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_core_utils.py", line 205, in _walk
    schema = self._schema_type_to_method[schema['type']](schema.copy(), f)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_core_utils.py", line 235, in handle_definitions_schema
    new_inner_schema = self.walk(schema['schema'], f)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_core_utils.py", line 202, in walk
    return f(schema, self._walk)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_discriminated_union.py", line 54, in inner
    s = apply_discriminator(s, discriminator, definitions)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_discriminated_union.py", line 86, in apply_discriminator
    return _ApplyInferredDiscriminator(discriminator, definitions or {}).apply(schema)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_discriminated_union.py", line 181, in apply
    schema = self._apply_to_root(schema)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_discriminated_union.py", line 221, in _apply_to_root
    self._handle_choice(choice)
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_discriminated_union.py", line 296, in _handle_choice
    self._set_unique_choice_for_values(choice, inferred_discriminator_values)
  File "/Users/fvoron/Development/polar/server/.venv/lib/python3.11/site-packages/pydantic/_internal/_discriminated_union.py", line 491, in _set_unique_choice_for_values
    raise TypeError(
TypeError: Value 'review_request_removed' for discriminator 'action' mapped to multiple choices

Additional context

The problem seems to come from here:

Union[
PullRequestReviewRequestRemovedOneof0, PullRequestReviewRequestRemovedOneof1
],
Union[PullRequestReviewRequestedOneof0, PullRequestReviewRequestedOneof1],

Pydantic doesn't like to have multiple choices for the same action. Apparently, this was working with Pydantic V1 (don't know how, the behavior was probably unexpected).

I don't really see how we could solve that, given that the action is the same; the only difference being on the presence of the fields requested_team/requested_reviewer. Maybe the simplest way could could be to tweak the generator to merge those two models?

@yanyongyu
Copy link
Owner

It seems this could be solved by functional discriminator which will be implemented in pydantic (already implemented in pydantic-core) pydantic/pydantic#7462.

@yanyongyu
Copy link
Owner

After reading some of the pydantic v2 source code, the discriminator extract and extend the choices when the schema is union_schema:

https://github.com/pydantic/pydantic/blob/9ab33eb82d78c55d1e66784cbd86107c1c41943d/pydantic/_internal/_discriminated_union.py#L260

This is the root cause of the error. We may need to apply different tag to the models and apply the choice by custom discriminator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working WebHook
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants