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 665C746121; Fri, 24 Jan 2025 12:39:49 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CE91542788; Fri, 24 Jan 2025 12:39:27 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id EE1C641144 for ; Fri, 24 Jan 2025 12:39:22 +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 74390497; Fri, 24 Jan 2025 03:39:50 -0800 (PST) Received: from localhost.localdomain (unknown [10.57.77.230]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 908193F694; Fri, 24 Jan 2025 03:39:21 -0800 (PST) From: Luca Vizzarro To: dev@dpdk.org Cc: Luca Vizzarro , Nicholas Pratte , Paul Szczepanek , Dean Marx , Patrick Robb Subject: [PATCH v4 5/7] dts: handle CLI overrides in the configuration Date: Fri, 24 Jan 2025 11:39:07 +0000 Message-ID: <20250124113909.137128-6-luca.vizzarro@arm.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250124113909.137128-1-luca.vizzarro@arm.com> References: <20240613201831.9748-3-npratte@iol.unh.edu> <20250124113909.137128-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 Reviewed-by: Dean Marx --- dts/framework/config/__init__.py | 65 ++++++++++++++++++++++++++++++-- dts/framework/runner.py | 18 ++------- 2 files changed, 64 insertions(+), 19 deletions(-) diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py index 2496f48e20..6ae98d0387 100644 --- a/dts/framework/config/__init__.py +++ b/dts/framework/config/__init__.py @@ -33,10 +33,11 @@ """ import tarfile +from collections.abc import Callable, MutableMapping 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 +45,60 @@ 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_fields_from_settings( + *fields: str | tuple[str, str], +) -> Callable[[Any, ValidationInfo], Any]: + """Before model validator that injects values from :attr:`ValidationContext.settings`. + + Args: + *fields: The name of the fields to apply the argument value to. If the settings field name + is not the same as the configuration field, supply a tuple with the respective names. + + Returns: + Pydantic before model validator. + """ + + def _loader(data: Any, info: ValidationInfo) -> Any: + if not isinstance(data, MutableMapping): + return data + + settings = cast(ValidationContext, info.context)["settings"] + for field in fields: + if isinstance(field, tuple): + settings_field = field[0] + config_field = field[1] + else: + settings_field = config_field = field + + if settings_data := getattr(settings, settings_field): + data[config_field] = settings_data + + return data + + return _loader + + class FrozenModel(BaseModel): """A pre-configured :class:`~pydantic.BaseModel`.""" @@ -317,6 +360,10 @@ class BaseDPDKBuildConfiguration(FrozenModel): #: The location of the DPDK tree. dpdk_location: DPDKLocation + dpdk_location_from_settings = model_validator(mode="before")( + load_fields_from_settings("dpdk_location") + ) + class DPDKPrecompiledBuildConfiguration(BaseDPDKBuildConfiguration): """DPDK precompiled build configuration.""" @@ -325,6 +372,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 = model_validator(mode="before")( + load_fields_from_settings("precompiled_build_dir") + ) + class DPDKBuildOptionsConfiguration(FrozenModel): """DPDK build options configuration. @@ -439,6 +490,10 @@ class TestRunConfiguration(FrozenModel): #: The seed to use for pseudo-random generation. random_seed: int | None = None + fields_from_settings = model_validator(mode="before")( + load_fields_from_settings("test_suites", "random_seed") + ) + class TestRunWithNodesConfiguration(NamedTuple): """Tuple containing the configuration of the test run and its associated nodes.""" @@ -541,7 +596,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 +607,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 +615,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