On Fri, Jul 29, 2022 at 10:55:47AM +0000, Juraj Linkeš wrote:
> From: Owen Hilyard <ohily...@iol.unh.edu>
>
> The configuration is split into two parts, one defining the parameters
> of the test run and the other defining the topology to be used.
>
> The format of the configuration is YAML. It is validated according to a
> json schema which also servers as detailed documentation of the various
s/servers/serves/
> configuration fields. This means that the complete set of allowed values
> are tied to the schema as a source of truth. This enables making changes
> to parts of DTS that interface with config files without a high risk of
> breaking someone's configuration.
>
> This configuration system uses immutable objects to represent the
> configuration, making IDE/LSP autocomplete work properly.
>
> There are two ways to specify the configuration file path, an
> environment variable or a command line argument, applied in that order.
>
> Signed-off-by: Owen Hilyard <ohily...@iol.unh.edu>
> Signed-off-by: Juraj Linkeš <juraj.lin...@pantheon.tech>
> ---
> dts/conf.yaml | 7 ++
> dts/framework/config/__init__.py | 99 ++++++++++++++++++++++
> dts/framework/config/conf_yaml_schema.json | 73 ++++++++++++++++
> dts/framework/exception.py | 14 +++
> dts/framework/settings.py | 65 ++++++++++++++
> 5 files changed, 258 insertions(+)
> create mode 100644 dts/conf.yaml
> create mode 100644 dts/framework/config/__init__.py
> create mode 100644 dts/framework/config/conf_yaml_schema.json
> create mode 100644 dts/framework/settings.py
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> new file mode 100644
> index 0000000000..cb12ea3d0f
> --- /dev/null
> +++ b/dts/conf.yaml
> @@ -0,0 +1,7 @@
> +executions:
> + - system_under_test: "SUT 1"
> +nodes:
> + - name: "SUT 1"
> + hostname: "SUT IP address or hostname"
> + user: root
> + password: "Leave blank to use SSH keys"
> diff --git a/dts/framework/config/__init__.py
> b/dts/framework/config/__init__.py
> new file mode 100644
> index 0000000000..a0fdffcd77
> --- /dev/null
> +++ b/dts/framework/config/__init__.py
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2010-2021 Intel Corporation
> +# Copyright(c) 2022 University of New Hampshire
> +#
> +
> +"""
> +Generic port and topology nodes configuration file load function
> +"""
> +import json
> +import os.path
> +import pathlib
> +from dataclasses import dataclass
> +from typing import Any, Optional
> +
> +import warlock
> +import yaml
> +
> +from framework.settings import SETTINGS
> +
> +
> +# Slots enables some optimizations, by pre-allocating space for the defined
> +# attributes in the underlying data structure.
> +#
> +# Frozen makes the object immutable. This enables further optimizations,
> +# and makes it thread safe should we every want to move in that direction.
> +@dataclass(slots=True, frozen=True)
> +class NodeConfiguration:
> + name: str
> + hostname: str
> + user: str
> + password: Optional[str]
> +
> + @staticmethod
> + def from_dict(d: dict) -> "NodeConfiguration":
> + return NodeConfiguration(
> + name=d["name"],
> + hostname=d["hostname"],
> + user=d["user"],
> + password=d.get("password"),
> + )
> +
Out of curiosity, what is the reason for having a static "from_dict" method
rather than just a regular constructor function that takes a dict as
parameter?
> +
> +@dataclass(slots=True, frozen=True)
> +class ExecutionConfiguration:
> + system_under_test: NodeConfiguration
> +
Minor comment: seems strange having only a single member variable in this
class, effectively duplicating the class above.
> + @staticmethod
> + def from_dict(d: dict, node_map: dict) -> "ExecutionConfiguration":
from reading the code it appears that node_map is a dict of
NodeConfiguration objects, right? Might be worth adding that to the
definition for clarity, and also the specific type of the dict "d" (if it
has one)
> + sut_name = d["system_under_test"]
> + assert sut_name in node_map, f"Unknown SUT {sut_name} in execution
> {d}"
> +
> + return ExecutionConfiguration(
> + system_under_test=node_map[sut_name],
> + )
> +
> +
> +@dataclass(slots=True, frozen=True)
> +class Configuration:
> + executions: list[ExecutionConfiguration]
> +
> + @staticmethod
> + def from_dict(d: dict) -> "Configuration":
> + nodes: list[NodeConfiguration] = list(
> + map(NodeConfiguration.from_dict, d["nodes"])
So "d" is a dict of dicts?
> + )
> + assert len(nodes) > 0, "There must be a node to test"
> +
> + node_map = {node.name: node for node in nodes}
> + assert len(nodes) == len(node_map), "Duplicate node names are not
> allowed"
> +
> + executions: list[ExecutionConfiguration] = list(
> + map(
> + ExecutionConfiguration.from_dict, d["executions"], [node_map
> for _ in d]
> + )
> + )
> +
> + return Configuration(executions=executions)
> +
> +
> +def load_config() -> Configuration:
> + """
> + Loads the configuration file and the configuration file schema,
> + validates the configuration file, and creates a configuration object.
> + """
> + with open(SETTINGS.config_file_path, "r") as f:
> + config_data = yaml.safe_load(f)
> +
> + schema_path = os.path.join(
> + pathlib.Path(__file__).parent.resolve(), "conf_yaml_schema.json"
> + )
> +
> + with open(schema_path, "r") as f:
> + schema = json.load(f)
> + config: dict[str, Any] = warlock.model_factory(schema,
> name="_Config")(config_data)
> + config_obj: Configuration = Configuration.from_dict(dict(config))
> + return config_obj
> +
> +
> +CONFIGURATION = load_config()
<snip>
