DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
Cc: <thomas@monjalon.net>, <david.marchand@redhat.com>,
	<ronan.randles@intel.com>, <Honnappa.Nagarahalli@arm.com>,
	<ohilyard@iol.unh.edu>, <lijuan.tu@intel.com>, <dev@dpdk.org>
Subject: Re: [PATCH v4 6/9] dts: add config parser module
Date: Tue, 13 Sep 2022 18:19:34 +0100	[thread overview]
Message-ID: <YyC7pn+pjjAunL4P@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20220729105550.1382664-7-juraj.linkes@pantheon.tech>

On Fri, Jul 29, 2022 at 10:55:47AM +0000, Juraj Linkeš wrote:
> From: Owen Hilyard <ohilyard@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 <ohilyard@iol.unh.edu>
> Signed-off-by: Juraj Linkeš <juraj.linkes@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>

  parent reply	other threads:[~2022-09-13 17:19 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-22 12:14 [PATCH v1 0/8] dts: ssh connection to a node Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 1/8] dts: add ssh pexpect library Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 2/8] dts: add locks for parallel node connections Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 3/8] dts: add ssh connection extension Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 4/8] dts: add basic logging facility Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 5/8] dts: add Node base class Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 6/8] dts: add config parser module Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 7/8] dts: add dts runtime workflow module Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 8/8] dts: add main script for running dts Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 0/8] ssh connection to a node Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 1/8] dts: add basic logging facility Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 2/8] dts: add ssh pexpect library Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 3/8] dts: add locks for parallel node connections Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 4/8] dts: add ssh connection extension Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 5/8] dts: add config parser module Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 6/8] dts: add Node base class Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 7/8] dts: add dts workflow module Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 8/8] dts: add dts executable script Juraj Linkeš
2022-07-28 10:00   ` [PATCH v3 0/9] dts: ssh connection to a node Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 1/9] dts: add project tools config Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 2/9] dts: add developer tools Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 3/9] dts: add basic logging facility Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 4/9] dts: add ssh pexpect library Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 5/9] dts: add ssh connection extension Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 6/9] dts: add config parser module Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 7/9] dts: add Node base class Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 8/9] dts: add dts workflow module Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 9/9] dts: add dts executable script Juraj Linkeš
2022-07-29 10:55     ` [PATCH v4 0/9] dts: ssh connection to a node Juraj Linkeš
2022-07-29 10:55       ` [PATCH v4 1/9] dts: add project tools config Juraj Linkeš
2022-08-10  6:30         ` Tu, Lijuan
2022-09-07 16:16         ` Bruce Richardson
2022-09-09 13:38           ` Juraj Linkeš
2022-09-09 13:52             ` Bruce Richardson
2022-09-09 14:13               ` Juraj Linkeš
2022-09-12 14:06                 ` Owen Hilyard
2022-09-12 15:15                   ` Bruce Richardson
2022-09-13 12:08                     ` Juraj Linkeš
2022-09-13 14:18                       ` Bruce Richardson
2022-09-13 19:03                     ` Honnappa Nagarahalli
2022-09-13 19:19                 ` Honnappa Nagarahalli
2022-09-14  9:37                   ` Thomas Monjalon
2022-09-14 12:55                     ` Juraj Linkeš
2022-09-14 13:11                       ` Bruce Richardson
2022-09-14 14:28                         ` Thomas Monjalon
2022-09-21 10:49                           ` Juraj Linkeš
2022-09-13 19:11             ` Honnappa Nagarahalli
2022-07-29 10:55       ` [PATCH v4 2/9] dts: add developer tools Juraj Linkeš
2022-08-10  6:30         ` Tu, Lijuan
2022-09-07 16:37         ` Bruce Richardson
2022-09-13 12:38           ` Juraj Linkeš
2022-09-13 20:38             ` Honnappa Nagarahalli
2022-09-14  7:37               ` Bruce Richardson
2022-09-14 12:45               ` Juraj Linkeš
2022-09-14 13:13                 ` Bruce Richardson
2022-09-14 14:26                   ` Thomas Monjalon
2022-09-14 19:08                     ` Honnappa Nagarahalli
2022-09-20 12:14                       ` Juraj Linkeš
2022-09-20 12:22                         ` Tu, Lijuan
2022-07-29 10:55       ` [PATCH v4 3/9] dts: add basic logging facility Juraj Linkeš
2022-08-10  6:31         ` Tu, Lijuan
2022-09-08  8:31         ` Bruce Richardson
2022-09-13 12:52           ` Juraj Linkeš
2022-09-13 23:31             ` Honnappa Nagarahalli
2022-09-14 12:51               ` Juraj Linkeš
2022-07-29 10:55       ` [PATCH v4 4/9] dts: add ssh pexpect library Juraj Linkeš
2022-08-10  6:31         ` Tu, Lijuan
2022-09-08  9:53         ` Bruce Richardson
2022-09-13 13:36           ` Juraj Linkeš
2022-09-13 14:23             ` Bruce Richardson
2022-09-13 14:59         ` Stanislaw Kardach
2022-09-13 17:23           ` Owen Hilyard
2022-09-14  0:03             ` Honnappa Nagarahalli
2022-09-14  7:42               ` Bruce Richardson
2022-09-14  7:58                 ` Stanislaw Kardach
2022-09-14 19:57                 ` Honnappa Nagarahalli
2022-09-19 14:21                   ` Owen Hilyard
2022-09-20 17:54                     ` Honnappa Nagarahalli
2022-09-21  1:01                       ` Tu, Lijuan
2022-09-21  5:37                       ` Jerin Jacob
2022-09-22  9:03                         ` Juraj Linkeš
2022-09-14  9:42         ` Stanislaw Kardach
2022-09-22  9:41           ` Juraj Linkeš
2022-09-22 14:32             ` Stanislaw Kardach
2022-09-23  7:22               ` Juraj Linkeš
2022-09-23  8:15                 ` Bruce Richardson
2022-09-23 10:18                   ` Stanislaw Kardach
2022-07-29 10:55       ` [PATCH v4 5/9] dts: add ssh connection extension Juraj Linkeš
2022-08-10  6:32         ` Tu, Lijuan
2022-09-13 17:04         ` Bruce Richardson
2022-09-13 17:32           ` Owen Hilyard
2022-09-14  7:46             ` Bruce Richardson
2022-09-14 12:02               ` Owen Hilyard
2022-09-14 13:15                 ` Bruce Richardson
2022-07-29 10:55       ` [PATCH v4 6/9] dts: add config parser module Juraj Linkeš
2022-08-10  6:33         ` Tu, Lijuan
2022-09-13 17:19         ` Bruce Richardson [this message]
2022-09-13 17:47           ` Owen Hilyard
2022-09-14  7:48             ` Bruce Richardson
2022-07-29 10:55       ` [PATCH v4 7/9] dts: add Node base class Juraj Linkeš
2022-08-10  6:33         ` Tu, Lijuan
2022-07-29 10:55       ` [PATCH v4 8/9] dts: add dts workflow module Juraj Linkeš
2022-08-10  6:34         ` Tu, Lijuan
2022-07-29 10:55       ` [PATCH v4 9/9] dts: add dts executable script Juraj Linkeš
2022-08-10  6:35         ` Tu, Lijuan

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=YyC7pn+pjjAunL4P@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=juraj.linkes@pantheon.tech \
    --cc=lijuan.tu@intel.com \
    --cc=ohilyard@iol.unh.edu \
    --cc=ronan.randles@intel.com \
    --cc=thomas@monjalon.net \
    /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).