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 D40854608B; Wed, 15 Jan 2025 15:20:05 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 891CF40611; Wed, 15 Jan 2025 15:19:41 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 2983B402EF for ; Wed, 15 Jan 2025 15:19:38 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id F077912FC; Wed, 15 Jan 2025 06:20:05 -0800 (PST) Received: from localhost.localdomain (JR4XG4HTQC.cambridge.arm.com [10.1.39.16]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id C90083F63F; Wed, 15 Jan 2025 06:19:36 -0800 (PST) From: Luca Vizzarro To: dev@dpdk.org Cc: Nicholas Pratte , Luca Vizzarro , Paul Szczepanek , Patrick Robb Subject: [PATCH v3 5/7] dts: handle CLI overrides in the configuration Date: Wed, 15 Jan 2025 14:18:07 +0000 Message-ID: <20250115141809.3898708-6-luca.vizzarro@arm.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250115141809.3898708-1-luca.vizzarro@arm.com> References: <20240705171341.23894-2-npratte@iol.unh.edu> <20250115141809.3898708-1-luca.vizzarro@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 The current handling of the configuration loading is inconsistent. After the whole configuration is loaded, if there are any CLI or environment overrides set, the code forcefully modifies the frozen configuration to use them. This changes the handling by passing the environment/CLI settings as part of the configuration context and handle the overrides directly at the field level before these are validated. As a positive side effect, the validator won't complain if a field is missing from the file but it has been specified as an environment/CLI override. Bugzilla ID: 1360 Bugzilla ID: 1598 Signed-off-by: Nicholas Pratte Signed-off-by: Luca Vizzarro Reviewed-by: Paul Szczepanek --- dts/framework/config/__init__.py | 40 ++++++++++++++++++++++++++++---- dts/framework/runner.py | 18 +++----------- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py index 2496f48e20..5510e3547b 100644 --- a/dts/framework/config/__init__.py +++ b/dts/framework/config/__init__.py @@ -36,7 +36,7 @@ from enum import Enum, auto, unique from functools import cached_property from pathlib import Path, PurePath -from typing import TYPE_CHECKING, Annotated, Any, Literal, NamedTuple +from typing import TYPE_CHECKING, Annotated, Any, Literal, NamedTuple, TypedDict, cast import yaml from pydantic import ( @@ -44,18 +44,36 @@ ConfigDict, Field, ValidationError, + ValidationInfo, field_validator, model_validator, ) from typing_extensions import Self from framework.exception import ConfigurationError +from framework.settings import Settings from framework.utils import REGEX_FOR_PCI_ADDRESS, StrEnum if TYPE_CHECKING: from framework.test_suite import TestSuiteSpec +class ValidationContext(TypedDict): + """A context dictionary to use for validation.""" + + #: The command line settings. + settings: Settings + + +def load_from_settings(data: Any, info: ValidationInfo): + """Before field validator that injects values from :attr:`ValidationContext.settings`.""" + context = cast(ValidationContext, info.context) + assert info.field_name is not None, "This validator can only be used as a field validator." + if settings_data := getattr(context["settings"], info.field_name): + return settings_data + return data + + class FrozenModel(BaseModel): """A pre-configured :class:`~pydantic.BaseModel`.""" @@ -317,6 +335,10 @@ class BaseDPDKBuildConfiguration(FrozenModel): #: The location of the DPDK tree. dpdk_location: DPDKLocation + dpdk_location_from_settings = field_validator("dpdk_location", mode="before")( + load_from_settings + ) + class DPDKPrecompiledBuildConfiguration(BaseDPDKBuildConfiguration): """DPDK precompiled build configuration.""" @@ -325,6 +347,10 @@ class DPDKPrecompiledBuildConfiguration(BaseDPDKBuildConfiguration): #: subdirectory of `~dpdk_location.dpdk_tree` or `~dpdk_location.tarball` root directory. precompiled_build_dir: str = Field(min_length=1) + build_dir_from_settings = field_validator("precompiled_build_dir", mode="before")( + load_from_settings + ) + class DPDKBuildOptionsConfiguration(FrozenModel): """DPDK build options configuration. @@ -439,6 +465,10 @@ class TestRunConfiguration(FrozenModel): #: The seed to use for pseudo-random generation. random_seed: int | None = None + fields_from_settings = field_validator("test_suites", "random_seed", mode="before")( + load_from_settings + ) + class TestRunWithNodesConfiguration(NamedTuple): """Tuple containing the configuration of the test run and its associated nodes.""" @@ -541,7 +571,7 @@ def validate_test_runs_with_nodes(self) -> Self: return self -def load_config(config_file_path: Path) -> Configuration: +def load_config(settings: Settings) -> Configuration: """Load DTS test run configuration from a file. Load the YAML test run configuration file, validate it, and create a test run configuration @@ -552,6 +582,7 @@ def load_config(config_file_path: Path) -> Configuration: Args: config_file_path: The path to the YAML test run configuration file. + settings: The settings provided by the user on the command line. Returns: The parsed test run configuration. @@ -559,10 +590,11 @@ def load_config(config_file_path: Path) -> Configuration: Raises: ConfigurationError: If the supplied configuration file is invalid. """ - with open(config_file_path, "r") as f: + with open(settings.config_file_path, "r") as f: config_data = yaml.safe_load(f) try: - return Configuration.model_validate(config_data) + context = ValidationContext(settings=settings) + return Configuration.model_validate(config_data, context=context) except ValidationError as e: raise ConfigurationError("failed to load the supplied configuration") from e diff --git a/dts/framework/runner.py b/dts/framework/runner.py index 0cdbb07e06..e46a8c1a4f 100644 --- a/dts/framework/runner.py +++ b/dts/framework/runner.py @@ -31,7 +31,6 @@ from .config import ( Configuration, - DPDKPrecompiledBuildConfiguration, SutNodeConfiguration, TestRunConfiguration, TestSuiteConfig, @@ -82,7 +81,7 @@ class DTSRunner: def __init__(self): """Initialize the instance with configuration, logger, result and string constants.""" - self._configuration = load_config(SETTINGS.config_file_path) + self._configuration = load_config(SETTINGS) self._logger = get_dts_logger() if not os.path.exists(SETTINGS.output_dir): os.makedirs(SETTINGS.output_dir) @@ -142,9 +141,7 @@ def run(self) -> None: self._init_random_seed(test_run_config) test_run_result = self._result.add_test_run(test_run_config) # we don't want to modify the original config, so create a copy - test_run_test_suites = list( - SETTINGS.test_suites if SETTINGS.test_suites else test_run_config.test_suites - ) + test_run_test_suites = test_run_config.test_suites if not test_run_config.skip_smoke_tests: test_run_test_suites[:0] = [TestSuiteConfig(test_suite="smoke_tests")] try: @@ -320,15 +317,6 @@ def _run_test_run( test_run_result.sut_info = sut_node.node_info try: dpdk_build_config = test_run_config.dpdk_config - if new_location := SETTINGS.dpdk_location: - dpdk_build_config = dpdk_build_config.model_copy( - update={"dpdk_location": new_location} - ) - if dir := SETTINGS.precompiled_build_dir: - dpdk_build_config = DPDKPrecompiledBuildConfiguration( - dpdk_location=dpdk_build_config.dpdk_location, - precompiled_build_dir=dir, - ) sut_node.set_up_test_run(test_run_config, dpdk_build_config) test_run_result.dpdk_build_info = sut_node.get_dpdk_build_info() tg_node.set_up_test_run(test_run_config, dpdk_build_config) @@ -622,6 +610,6 @@ def _exit_dts(self) -> None: def _init_random_seed(self, conf: TestRunConfiguration) -> None: """Initialize the random seed to use for the test run.""" - seed = SETTINGS.random_seed or conf.random_seed or random.randrange(0xFFFF_FFFF) + seed = conf.random_seed or random.randrange(0xFFFF_FFFF) self._logger.info(f"Initializing test run with random seed {seed}.") random.seed(seed) -- 2.43.0