Skip to content

Conversation

@Leobouloc
Copy link
Contributor

@Leobouloc Leobouloc commented Aug 11, 2023

Purpose

As mentionned in #401 , upgrading to pydantic 2 should greatly improve performances. Also it would allow projects depending on ralph (such as warren) to upgrade more easily.

Dealing with common errors when migrating

ERROR: Extra not permitted

In conf.py, errors such as:

    super().__init__(
E   pydantic_core._pydantic_core.ValidationError: 1 validation error for Settings
E   LOCALE_ENCODING
E     Extra inputs are not permitted [type=extra_forbidden, input_value='utf-8', input_type=str]
E       For further information visit https://errors.pydantic.dev/2.1/v/extra_forbidden

may be due to a change in default behavior in models extra=ignore to extra=failure. We may set this parameter manually to fix this issue.

ERROR: Unable to generate pydantic-core schema

E   pydantic.errors.PydanticSchemaGenerationError: Unable to generate pydantic-core schema for <class 'ralph.models.xapi.base.common.IRI'>. Set `arbitrary_types_allowed=True` in the model_config to ignore this error or implement `__get_pydantic_core_schema__` on your type to fully support it.
E
E   If you got this error by calling handler(<some type>) within `__get_pydantic_core_schema__` then you likely need to call `handler.generate_schema(<some type>)` since we do not call `__get_pydantic_core_schema__` on `<some type>` otherwise to avoid infinite recursion.
E

To fix this we change the way we define classes.

Previously in Ralph

class CommaSeparatedTuple(BaseModel):
    @model_validator(mode='before')
    @classmethod
    def validate(cls, value: Union[str, Tuple[str]]) -> Tuple[str]:
        """Checks whether the value is a comma separated string or a tuple."""

        if isinstance(value, tuple):
            return value

        if isinstance(value, str):
            return tuple(value.split(","))

        raise TypeError("Invalid comma separated list")

New in Ralph

def validate_comma_separated_tuple(value: Union[str, Tuple[str, ...]]) -> Tuple[str]:
    """Checks whether the value is a comma separated string or a tuple."""

    if isinstance(value, tuple):
        return value

    if isinstance(value, str):
        return tuple(value.split(","))

    raise TypeError("Invalid comma separated list")

CommaSeparatedTuple = Annotated[Union[str, Tuple[str, ...]], AfterValidator(validate_comma_separated_tuple)]

ERROR: Missing values

A recurrent error is the one below, which should be linked to the change of behavior when using Optional[...] in Pydantic V2. Previously, it would assign a default value of None, which is no longer the case. This should have been dealt with by bump-pydantic but was not always the case. It would be nice to know why before confirming this solution.

src/ralph/models/xapi/video/statements.py:85: in VideoPlayed
    verb: PlayedVerb = PlayedVerb()
E   pydantic_core._pydantic_core.ValidationError: 1 validation error for PlayedVerb
E   display
E     Field required [type=missing, input_value={}, input_type=dict]
E       For further information visit https://errors.pydantic.dev/2.1/v/missing

Previously in Ralph

class VideoActivity(BaseXapiActivity):
    name: Optional[Dict[Literal[LANG_EN_US_DISPLAY], str]]

New in Ralph

class VideoActivity(BaseXapiActivity):
    name: Optional[Dict[Literal[LANG_EN_US_DISPLAY], str]] = None

@Leobouloc Leobouloc changed the title Upgrade to Pydantic v2 [WIP] Upgrade to Pydantic v2 Aug 11, 2023
@Leobouloc Leobouloc added the WIP label Aug 11, 2023
@wilbrdt wilbrdt added this to the 5.0 milestone Aug 16, 2023
; library (mostly models).
langcodes>=3.2.0
pydantic[dotenv,email]>=1.10.0, <2.0
pydantic[email]>=2.1.1, <3.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pydantic[email]>=2.1.1, <3.0
pydantic[email]>=2.1.1


class ESDatabaseBackendSettings(InstantiableSettingsItem):
"""Pydantic modelf for Elasticsearch database backend configuration settings."""
"""Pydantic modelf for Elasticsearch database backend configuration settings."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""Pydantic modelf for Elasticsearch database backend configuration settings."""
"""Pydantic model for Elasticsearch database backend configuration settings."""

Copy link
Contributor

@quitterie-lcs quitterie-lcs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

from enum import Enum
from pathlib import Path
from typing import List, Tuple, Union
from typing import Any, List, Tuple, Union
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from typing import Any, List, Tuple, Union
from typing import Any, List, Optional, Tuple, Union


try:
from typing import Literal
from typing import Annotated, Literal, Optional
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from typing import Annotated, Literal, Optional
from typing import Annotated, Literal

from typing import Annotated, Literal, Optional
except ImportError:
from typing_extensions import Literal
from typing_extensions import Annotated, Literal, Optional
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from typing_extensions import Annotated, Literal, Optional
from typing_extensions import Annotated, Literal

; library (mostly models).
langcodes>=3.2.0
pydantic[dotenv,email]>=1.10.0, <2.0
pydantic[email]>=2.1.1, <3.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pydantic[email]>=2.1.1, <3.0
pydantic[email]>=2.2.0

@Leobouloc
Copy link
Contributor Author

replaced by #504

@Leobouloc Leobouloc closed this Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

5 participants