[v4,5/7] dts: handle CLI overrides in the configuration
Checks
Commit Message
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>
Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
---
dts/framework/config/__init__.py | 65 ++++++++++++++++++++++++++++++--
dts/framework/runner.py | 18 ++-------
2 files changed, 64 insertions(+), 19 deletions(-)
Comments
Thank you for addressing this. Great work!
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
On Fri, Jan 24, 2025 at 6:39 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> 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>
> Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
> ---
> 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
>
@@ -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
@@ -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)