yanyongyu / githubkit

The modern, all-batteries-included GitHub SDK for Python, including rest api, graphql, webhooks, like octokit!
MIT License
159 stars 21 forks source link

Support Pydantic v2 #38

Closed dosisod closed 10 months ago

dosisod commented 1 year ago

Pydantic v2 just came out, so it would be nice if githubkit could support the newer, faster version of Pydantic. I attempted to upgrade it myself, but there are a few bugs and non-trivial design decisions that would need to be made to support Pydantic v2, so I thought I'd bring this up now.

The migration guide does a good job of explaining what's changed, and how to upgrade your code. I'm just showing what I've encountered thus far, though there is probably additional work that needs to be done.

First up, regex is being renamed to pattern in Field() objects. Easy enough to fix, just change it in the codegen.

Second thing I ran into is that parse_raw_as has been removed, and instead you need to use a TypeAdapter:

    @property
    def parsed_data(self) -> RT:
        # old
        return parse_raw_as(self._data_model, self._response.content)

        # new
        return TypeAdapter(self._data_model).validate_python(self._response.content)

Lastly (or at least, where I stopped) is with Missing[]. This is a bug, as Pydantic v2 does not seem to like literal values like Literal[UNSET]. For example, the following code is throwing an error:

class Dependency(GitHubRestModel):
    """Dependency"""

    package_url: Missing[str] = Field(
        description="Package-url (PURL) of dependency. See https://github.com/package-url/purl-spec for more details.",
        pattern="^pkg",
        default=UNSET,
    )

This produces:

...

  File "githubkit/rest/models.py", line 7876, in <module>
    class Dependency(GitHubRestModel):
  File ".venv/lib/python3.11/site-packages/pydantic/_internal/_model_construction.py", line 172, in __new__
    complete_model_class(
  File ".venv/lib/python3.11/site-packages/pydantic/_internal/_model_construction.py", line 438, in complete_model_class
    cls.__pydantic_validator__ = SchemaValidator(simplified_core_schema, core_config)
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pydantic_core._pydantic_core.SchemaError: Invalid Schema:
model.schema.model-fields.fields.package_url.schema.default.schema.union.pattern
  Extra inputs are not permitted [type=extra_forbidden, input_value='^pkg', input_type=str]
    For further information visit https://errors.pydantic.dev/2.1.2/v/extra_forbidden

Replacing Missing[str] with Union[Literal["xyz"], str] still fails, but Optional[str] works fine.

This seems to be tracked in https://github.com/pydantic/pydantic/issues/6601. The error looks slightly different, though it still is an issue with Literal values in Unions.

I'm happy to work on a PR for this, though I thought I would get your input on a few things before I go ahead:

Let me know what your thoughts are on this, thanks!

yanyongyu commented 1 year ago

The error reported with MISSING[str] is that the Union type does not allow to declare a pattern in the Field info. This seems different from pydantic/pydantic#6601, and may be a new issue. Here is a minimum repreduce code:

class Test(BaseModel):
    field: Union[int, str] = Field(default=1, pattern="^pkg")

This is not related to the UNSET and Literal, and can be repreduced using any types union. This field info is allowed in pydantic v1. May be we should report this to pydantic v2.

yanyongyu commented 1 year ago

After some experiment...

It works with the Annotated expression:

from typing import Union, Annotated
from pydantic import BaseModel, Field, ConfigDict

class Test(BaseModel):
    field: Union[int, Annotated[str, Field(pattern="^pkg")]] = Field(default=1)

    model_config = ConfigDict(strict=True)

I have reported this problem to pydantic issue.

yanyongyu commented 1 year ago

The same issue as https://github.com/pydantic/pydantic/issues/6577.

dosisod commented 1 year ago

I guess that makes sense, though I'd argue any object with a __str__() method should be usable with pattern. Might yield weird results, but feels more intuitive to me (and better than a vague error).

How trivial would it be to add codegen support for this Annotated fields? I imagine that there are params other than pattern that will need to be handled differently as well.

yanyongyu commented 1 year ago

Since the new constrant logic of pydantic v2, we may need to bind the constrants directly on the type instead of the field, and generate the type using Annotated. We should have a constrant-type mapping according to the json schema to do this bind operation.

yanyongyu commented 10 months ago

Thanks to @sudosubin, githubkit now supports pydantic v2. But, before this feature can be released, i need to investigate how to make it compatible with pydantic v1 and v2. I will refactor the codegen for future improvements including lazy loading #11 after another project i'm currently working on finished.