Re: [PATCH 3/5] dts: use Pydantic in the configuration

[Dean Marx <dmarx@iol.unh.edu>]

[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]

On Thu, Aug 22, 2024 at 12:40 PM Luca Vizzarro <luca.vizzarro@arm.com>
wrote:

> This change brings in Pydantic in place of Warlock. Pydantic offers
> a built-in model validation system in the classes, which allows for
> a more resilient and simpler code. As a consequence of this change:
>
> - most validation is now built-in
> - further validation is added to verify:
>   - cross referencing of node names and ports
>   - test suite and test cases names
> - dictionaries representing the config schema are removed
> - the config schema is no longer used for validation but kept as an
>   alternative format for the developer
> - the config schema can now be generated automatically from the
>   Pydantic models
> - the TrafficGeneratorType enum has been changed from inheriting
>   StrEnum to the native str and Enum. This change was necessary to
>   enable the discriminator for object unions
> - the structure of the classes has been slightly changed to perfectly
>   match the structure of the configuration files
> - updates the test suite argument to catch the ValidationError that
>   TestSuiteConfig can now raise
>
> Bugzilla ID: 1508
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>


 <snip>

> -@dataclass(slots=True, frozen=True)
> +@dataclass(slots=True, frozen=True, kw_only=True,
> config=ConfigDict(extra="forbid"))
>

Up to you but I think it might be worth specifying what some of these extra
pydantic args are for if we're going to keep the name of the decorator as
"dataclass." For example, this ConfigDict "forbid" argument seems to be
commonly used, same with the "before/after" modes with the model_validator.
Maybe a brief description somewhere in the docstrings, just so others can
see how it differs from the previous implementation even without experience
using pydantic.

<snip>

> +    @model_validator(mode="before")
>      @classmethod
> -    def from_dict(
> -        cls,
> -        entry: str | TestSuiteConfigDict,
> -    ) -> Self:
> -        """Create an instance from two different types.
> +    def convert_from_string(cls, data: Any) -> Any:
> +        """Convert the string representation into a valid mapping."""
> +        if isinstance(data, str):
> +            [test_suite, *test_cases] = data.split()
> +            return dict(test_suite=test_suite, test_cases=test_cases)
> +        return data
>

Again this is completely your call, but might be worth explaining in the
docstrings why this "before" method is used here while the other validators
are running with "after."

[-- Attachment #2: Type: text/html, Size: 3651 bytes --]