From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6C47745A6C; Mon, 30 Sep 2024 19:56:24 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EF7FE40614; Mon, 30 Sep 2024 19:56:23 +0200 (CEST) Received: from mail-lj1-f171.google.com (mail-lj1-f171.google.com [209.85.208.171]) by mails.dpdk.org (Postfix) with ESMTP id ED6654060C for ; Mon, 30 Sep 2024 19:56:21 +0200 (CEST) Received: by mail-lj1-f171.google.com with SMTP id 38308e7fff4ca-2fabe9fbe20so1113911fa.1 for ; Mon, 30 Sep 2024 10:56:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1727718981; x=1728323781; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=YuessqJETV1OuJ4ny1aNffLd9JrtSS1RBFILXy6GaKU=; b=fIXHElbNtARpbF+G7VdDOcWI1xiyd0+BFiH5aMWqE4xYP2sl28Bw7gKWdVsBttX5Xx wZYtpj+gKUC49VDxt+u1PB+BIrtbrOPbOaVVAug11Y9W38y7glo0H24luZO4Crxg2Dxc CNLrRYj91DBVbQ1R1X3dv30/s3lcBPG2I+8Nc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727718981; x=1728323781; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YuessqJETV1OuJ4ny1aNffLd9JrtSS1RBFILXy6GaKU=; b=gDK7TLrV5ZiFoXZR5xnrI115nM5hlMgLUOA+HQIsFW0f/aS4CUnYmWK6Z4nBig9Nfm MigPo75EXuz0xGAHkZaT9PGOH+Hk8m1qnE/RgydOmAw5OKk4Vk8sMCgj0BqsqmortqB6 6eZ8ccXgceKKVnI2Fsg/vfzVHoh8kkuF64iKoJhoLBKAfDt+BFLxHoXeuV8b7mmr8j1i ggM48GtfylGr9tlqAgHKHbxqBFICSIaerZ9DaghvuuTX75ci+l1nUNTBI0HeoIZRHb3F 0gJXxU72H1pdWRrit76bXZaDvMn9vAbC3Xp+Edyr5ADOCuDsobC/b3sa4QRuRDPix/eZ AX1w== X-Gm-Message-State: AOJu0YyQD3ZCQ00Ep4I1h8KYH4NOREaLNTZrCNAwzUe59e4lnC2/pU0s 0/PXVAJ8bAG6Bt4nGpa/I2LZrJZPH8G2z2h38zdxDzNQZiYlPKO5B4nHK0xcVKAhc7JKBZZbGZK fSf8RrkmxZIZzKRg/q9j7L/5sMG6uRKmlLMH8Fw== X-Google-Smtp-Source: AGHT+IH4s9bziDOJWtrjLNzQEowbwYKeUUjmV1vZOAhzQmh7at0GoKm965V0cErhKilJ3B7bok8lK4m3/kuch3xAiaA= X-Received: by 2002:a2e:be1b:0:b0:2fa:bf5f:f97e with SMTP id 38308e7fff4ca-2fabff718e4mr15327251fa.8.1727718981021; Mon, 30 Sep 2024 10:56:21 -0700 (PDT) MIME-Version: 1.0 References: <20240822163941.1390326-1-luca.vizzarro@arm.com> <20240822163941.1390326-4-luca.vizzarro@arm.com> In-Reply-To: <20240822163941.1390326-4-luca.vizzarro@arm.com> From: Nicholas Pratte Date: Mon, 30 Sep 2024 13:56:09 -0400 Message-ID: Subject: Re: [PATCH 3/5] dts: use Pydantic in the configuration To: Luca Vizzarro Cc: dev@dpdk.org, Honnappa Nagarahalli , =?UTF-8?Q?Juraj_Linke=C5=A1?= , Paul Szczepanek Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Luca! See my comments below, thanks! On Thu, Aug 22, 2024 at 12:40=E2=80=AFPM Luca Vizzarro 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 > Reviewed-by: Paul Szczepanek > --- > dts/framework/config/__init__.py | 588 +++++++++--------- > dts/framework/config/types.py | 132 ---- > dts/framework/runner.py | 35 +- > dts/framework/settings.py | 16 +- > dts/framework/testbed_model/sut_node.py | 2 +- > .../traffic_generator/__init__.py | 4 +- > .../traffic_generator/traffic_generator.py | 2 +- > 7 files changed, 325 insertions(+), 454 deletions(-) > delete mode 100644 dts/framework/config/types.py > > diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__in= it__.py > index df60a5030e..013c529829 100644 > --- a/dts/framework/config/__init__.py > +++ b/dts/framework/config/__init__.py > @@ -2,17 +2,19 @@ > # Copyright(c) 2010-2021 Intel Corporation > # Copyright(c) 2022-2023 University of New Hampshire > # Copyright(c) 2023 PANTHEON.tech s.r.o. > +# Copyright(c) 2024 Arm Limited > > """Testbed configuration and test suite specification. > > This package offers classes that hold real-time information about the te= stbed, hold test run > configuration describing the tested testbed and a loader function, :func= :`load_config`, which loads > -the YAML test run configuration file > -and validates it according to :download:`the schema `. > +the YAML test run configuration file and validates it against the :class= :`Configuration` Pydantic > +dataclass model. The Pydantic model is also available as Out of curiosity, what is the reason for maintaining use of dataclasses here as opposed to creating BaseModel subclasses for the Pydantic library? I suppose both implementations would lead to the same result, but is it mostly for the sake of familiarity and consistency that we're still using dataclasses here? > +:download:`JSON schema `. > > The YAML test run configuration file is parsed into a dictionary, parts = of which are used throughout > -this package. The allowed keys and types inside this dictionary are defi= ned in > -the :doc:`types ` module. > +this package. The allowed keys and types inside this dictionary map dire= ctly to the > +:class:`Configuration` model, its fields and sub-models. > > The test run configuration has two main sections: > > @@ -24,7 +26,7 @@ > > The real-time information about testbed is supposed to be gathered at ru= ntime. > > -The classes defined in this package make heavy use of :mod:`dataclasses`= . > +The classes defined in this package make heavy use of :mod:`pydantic.dat= aclasses`. > All of them use slots and are frozen: > > * Slots enables some optimizations, by pre-allocating space for the = defined > @@ -33,29 +35,31 @@ > and makes it thread safe should we ever want to move in that direc= tion. > """ > > -import json > -import os.path > -from dataclasses import dataclass, fields > -from enum import auto, unique > +from enum import Enum, auto, unique > +from functools import cached_property > from pathlib import Path > -from typing import Union > +from typing import TYPE_CHECKING, Annotated, Any, Literal, NamedTuple, P= rotocol > > -import warlock # type: ignore[import-untyped] > import yaml > +from pydantic import ( > + ConfigDict, > + Field, > + StringConstraints, > + TypeAdapter, > + ValidationError, > + field_validator, > + model_validator, > +) > +from pydantic.config import JsonDict > +from pydantic.dataclasses import dataclass > from typing_extensions import Self > > -from framework.config.types import ( > - BuildTargetConfigDict, > - ConfigurationDict, > - NodeConfigDict, > - PortConfigDict, > - TestRunConfigDict, > - TestSuiteConfigDict, > - TrafficGeneratorConfigDict, > -) > from framework.exception import ConfigurationError > from framework.utils import StrEnum > > +if TYPE_CHECKING: > + from framework.test_suite import TestSuiteSpec > + > > @unique > class Architecture(StrEnum): > @@ -116,14 +120,14 @@ class Compiler(StrEnum): > > > @unique > -class TrafficGeneratorType(StrEnum): > +class TrafficGeneratorType(str, Enum): > """The supported traffic generators.""" > > #: > - SCAPY =3D auto() > + SCAPY =3D "SCAPY" Going off of Juraj's comments, would you be able to provide an deeper explanation as how this new parameterization of str and enum works with respect to the Pydantic field discriminators? The rest of what you've provided is more straightforward, but I'd still like to look at this deeper and provide review. The comments asked here are probably my most significant.