DPDK patches and discussions
 help / color / mirror / Atom feed
From: Luca Vizzarro <luca.vizzarro@arm.com>
To: dev@dpdk.org
Cc: Nicholas Pratte <npratte@iol.unh.edu>,
	Luca Vizzarro <luca.vizzarro@arm.com>,
	Paul Szczepanek <paul.szczepanek@arm.com>,
	Patrick Robb <probb@iol.unh.edu>
Subject: [PATCH v3 5/7] dts: handle CLI overrides in the configuration
Date: Wed, 15 Jan 2025 14:18:07 +0000	[thread overview]
Message-ID: <20250115141809.3898708-6-luca.vizzarro@arm.com> (raw)
In-Reply-To: <20250115141809.3898708-1-luca.vizzarro@arm.com>

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 <npratte@iol.unh.edu>
Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
---
 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


  parent reply	other threads:[~2025-01-15 14:20 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-13 20:18 [PATCH 0/4] dts: Remove Excess Attributes From User Config Nicholas Pratte
2024-06-13 20:18 ` [PATCH 1/4] dts: Remove build target config and list of devices Nicholas Pratte
2024-06-14 18:07   ` Jeremy Spewock
2024-06-13 20:18 ` [PATCH 2/4] dts: Use First Core Logic Change Nicholas Pratte
2024-06-14 18:09   ` Jeremy Spewock
2024-06-20 13:41     ` Nicholas Pratte
2024-06-13 20:18 ` [PATCH 3/4] dts: Self-Discovering Architecture Change Nicholas Pratte
2024-06-14 18:09   ` Jeremy Spewock
2024-06-13 20:18 ` [PATCH 4/4] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
2024-06-14 18:11   ` Jeremy Spewock
2024-07-05 17:13 ` [PATCH v2 0/6] dts: Remove Excess Attributes From User Config Nicholas Pratte
2024-07-05 18:29   ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
2024-11-06 19:29     ` Dean Marx
2024-07-05 18:31   ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
2024-11-06 19:48     ` Dean Marx
2024-07-05 18:32   ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
2024-11-06 20:13     ` Dean Marx
2024-07-05 18:32   ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
2024-11-06 20:32     ` Dean Marx
2024-07-05 18:33   ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
2024-07-05 18:33   ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
2025-01-15 14:18   ` [PATCH v3 0/7] dts: refactor configuration Luca Vizzarro
2025-01-15 14:18     ` [PATCH v3 1/7] dts: enable arch self-discovery Luca Vizzarro
2025-01-15 14:18     ` [PATCH v3 2/7] dts: simplify build options config Luca Vizzarro
2025-01-15 14:18     ` [PATCH v3 3/7] dts: infer use first core without config Luca Vizzarro
2025-01-15 14:18     ` [PATCH v3 4/7] dts: rework DPDK attributes in SUT node config Luca Vizzarro
2025-01-15 14:18     ` Luca Vizzarro [this message]
2025-01-15 14:18     ` [PATCH v3 6/7] dts: split configuration file Luca Vizzarro
2025-01-15 14:18     ` [PATCH v3 7/7] dts: run all test suites by default Luca Vizzarro
2024-07-05 17:13 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte
2024-07-16 15:07   ` Jeremy Spewock
2024-09-12 20:33     ` Nicholas Pratte
2024-09-10 11:30   ` Juraj Linkeš
2024-09-12 20:31     ` Nicholas Pratte
2024-11-18 16:51   ` Luca Vizzarro
2024-07-05 17:13 ` [PATCH v2 2/6] dts: Use First Core Logic Change Nicholas Pratte
2024-09-10 13:34   ` Juraj Linkeš
2024-11-18 16:54   ` Luca Vizzarro
2024-07-05 17:13 ` [PATCH v2 3/6] dts: Self-Discovering Architecture Change Nicholas Pratte
2024-09-10 13:41   ` Juraj Linkeš
2024-11-18 17:14   ` Luca Vizzarro
2024-07-05 17:13 ` [PATCH v2 4/6] dts: Rework DPDK Attributes In SUT Node Config Nicholas Pratte
2024-09-10 14:04   ` Juraj Linkeš
2024-11-18 17:16   ` Luca Vizzarro
2024-07-05 17:13 ` [PATCH v2 5/6] dts: add conditional behavior for test suite Nicholas Pratte
2024-07-16 14:59   ` Jeremy Spewock
2024-09-10 14:12   ` Juraj Linkeš
2024-11-06 20:52   ` Dean Marx
2024-11-18 17:21   ` Luca Vizzarro
2024-07-05 17:13 ` [PATCH v2 6/6] doc: dpdk documentation changes for new dts config Nicholas Pratte
2024-09-10 14:17   ` Juraj Linkeš
2024-11-06 20:57   ` Dean Marx
2024-11-18 17:21   ` Luca Vizzarro
2024-07-05 18:24 ` [PATCH v2 1/6] dts: Remove build target config and list of devices Nicholas Pratte

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250115141809.3898708-6-luca.vizzarro@arm.com \
    --to=luca.vizzarro@arm.com \
    --cc=dev@dpdk.org \
    --cc=npratte@iol.unh.edu \
    --cc=paul.szczepanek@arm.com \
    --cc=probb@iol.unh.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).