Pydantic V2 - Error when parsing a `pull_request` webhook event

frankie567 commented 1 year ago

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:

https://github.com/yanyongyu/githubkit/blob/cead9fcfebf0e38de67bbeb755688b2b7dede188/githubkit/webhooks/types.py#L504-L507

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 commented 1 year ago

It seems this could be solved by functional discriminator which will be implemented in pydantic (already implemented in pydantic-core) https://github.com/pydantic/pydantic/issues/7462.

yanyongyu commented 10 months ago

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.

yanyongyu / githubkit