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 D558945A6D; Mon, 30 Sep 2024 23:44:54 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 784C140150; Mon, 30 Sep 2024 23:44:54 +0200 (CEST) Received: from mail-yw1-f171.google.com (mail-yw1-f171.google.com [209.85.128.171]) by mails.dpdk.org (Postfix) with ESMTP id D17BC4014F for ; Mon, 30 Sep 2024 23:44:52 +0200 (CEST) Received: by mail-yw1-f171.google.com with SMTP id 00721157ae682-6db4b602e38so39061347b3.1 for ; Mon, 30 Sep 2024 14:44:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1727732692; x=1728337492; darn=dpdk.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=ZtwOKuv9wYfG7rnYxhC/kFKQuWS5ncoy8ApKHARZUhI=; b=K0s9WQr02NYXuJ3FED9e56v9VAH6my53IluPFVMDsra4uPLPm6dwsknF7N1cqE1oxg AFi8SRs3v8whewNnFNLq3sqBLzzVwhfuOALWesfEo1L0ICc/2265h+xN7Ral+CSWMqma Q2kZqV7SVO9rgw5Juh/4DqeoAToBnNcqcp1+g= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727732692; x=1728337492; h=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=ZtwOKuv9wYfG7rnYxhC/kFKQuWS5ncoy8ApKHARZUhI=; b=fUzMaVTvJQicnlBDIgb+GRSyR/nWGPDzlDLZ7LC6cCTG+uYxYLyHa0UkNmEbe0Op1Y e6yP4GztuBKAMynpqSTNKzhQHTx5ODHOASaAJNJwnLhm+tlTiwDy747me89EcMiijK7k IoffWWknCOQkAaHmZvbbLFwps2G4IQcsxFqC4Q+F4uIw+PVSNTdXP1qaax9sooCJbzaX zNWJDx3rk9CpyplW5lJt7S2fCRAtHCn/yjJxrgNDqtXtLYB9qq1m8y6Hm9mzftrTlw3h F5+ScwJcleSUKwGclEqniNFv0rBd6ik82E4dNAn/DxJB10wdNXNAeiN+d8ce12EDqsuf i9YA== X-Gm-Message-State: AOJu0YxHGELzRNg+W+XYIC9aAXTs5ZYy7UAf2chIrGferAjmrT7epL+t J/KQu71IxLc/V968ObSobOxWavpwa4/rtWU7+fTJSY5rdAT1HCcluLJeEvXgcuViWK9QQv2JFXz AMcgAbXTopJuJlMq+NXn9OnKifdyViPyZs6zXfw== X-Google-Smtp-Source: AGHT+IEu+/uU0Dqe3j/8f4UkI6Cq7DPRbWeJcWiUsVUSfItSPry+fgL6JhpwM5OtC7WIEP4cSS6eS8FBxidz0dDBLK0= X-Received: by 2002:a05:690c:c0f:b0:6d3:d842:5733 with SMTP id 00721157ae682-6e2475e3050mr112037627b3.35.1727732692118; Mon, 30 Sep 2024 14:44:52 -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: Dean Marx Date: Mon, 30 Sep 2024 17:45:10 -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: multipart/alternative; boundary="0000000000006188b606235d1d24" 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 --0000000000006188b606235d1d24 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 > -@dataclass(slots=3DTrue, frozen=3DTrue) > +@dataclass(slots=3DTrue, frozen=3DTrue, kw_only=3DTrue, > config=3DConfigDict(extra=3D"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. > + @model_validator(mode=3D"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] =3D data.split() > + return dict(test_suite=3Dtest_suite, test_cases=3Dtest_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." --0000000000006188b606235d1d24 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Thu, Aug 22, 2024 at 12:40=E2=80=AFPM = Luca Vizzarro <luca.vizzarro@ar= m.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:
=C2=A0 - cross referencing of node names and ports
=C2=A0 - 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
=C2=A0 alternative format for the developer
- the config schema can now be generated automatically from the
=C2=A0 Pydantic models
- the TrafficGeneratorType enum has been changed from inheriting
=C2=A0 StrEnum to the native str and Enum. This change was necessary to
=C2=A0 enable the discriminator for object unions
- the structure of the classes has been slightly changed to perfectly
=C2=A0 match the structure of the configuration files
- updates the test suite argument to catch the ValidationError that
=C2=A0 TestSuiteConfig can now raise

Bugzilla ID: 1508

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>

=C2=A0<snip>
-@dataclass(slots=3DTrue, frozen=3DTrue)
+@dataclass(slots=3DTrue, frozen=3DTrue, kw_only=3DTrue, config=3DConfigDic= t(extra=3D"forbid"))

Up to yo= u but I think it might be worth specifying what some of these extra pydanti= c=C2=A0args are for if we're going to keep the name of the decorator as= "dataclass." For example, this ConfigDict "forbid" arg= ument seems to be commonly used, same with the "before/after" mod= es with the model_validator. Maybe a brief description somewhere in the doc= strings, just so others can see how it differs from the previous implementa= tion even without experience using pydantic.
=C2=A0
<= ;snip>=C2=A0
+=C2=A0 =C2=A0 @model_validator(mode=3D"before")
=C2=A0 =C2=A0 =C2=A0@classmethod
-=C2=A0 =C2=A0 def from_dict(
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 cls,
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 entry: str | TestSuiteConfigDict,
-=C2=A0 =C2=A0 ) -> Self:
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Create an instance from two = different types.
+=C2=A0 =C2=A0 def convert_from_string(cls, data: Any) -> Any:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Convert the string represent= ation into a valid mapping."""
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 if isinstance(data, str):
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 [test_suite, *test_cases] =3D da= ta.split()
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return dict(test_suite=3Dtest_su= ite, test_cases=3Dtest_cases)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 return data

Again this is completely your call, but mi= ght be worth explaining in the docstrings why this "before" metho= d is used here while the other validators are running with "after.&quo= t;
--0000000000006188b606235d1d24--