<div dir="ltr"><div><snip> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +# Frozen makes the object immutable. This enables further optimizations,<br>
> +# and makes it thread safe should we every want to move in that direction.<br>
> +@dataclass(slots=True, frozen=True)<br>
> +class NodeConfiguration:<br>
> +    name: str<br>
> +    hostname: str<br>
> +    user: str<br>
> +    password: Optional[str]<br>
> +<br>
> +    @staticmethod<br>
> +    def from_dict(d: dict) -> "NodeConfiguration":<br>
> +        return NodeConfiguration(<br>
> +            name=d["name"],<br>
> +            hostname=d["hostname"],<br>
> +            user=d["user"],<br>
> +            password=d.get("password"),<br>
> +        )<br>
> +<br>
Out of curiosity, what is the reason for having a static "from_dict" method<br>
rather than just a regular constructor function that takes a dict as<br>
parameter?<br></blockquote><div><br></div><div>@dataclass(...) is a class annotation that transforms the thing it annotates into a dataclass. This means it creates the constructor for you based on the property type annotations. If you create your own constructor, you need a constructor that can either take a single dictionary or all of the parameters like a normal constructor. Making it a static method also means that each class can manage how it should be constructed from a dictionary. Some of the other classes will transform lists or perform other assertions. It also makes it easier to have specialized types. For instance, a NICConfiguration class would have to handle all of the possible device arguments that could be passed to any PMD driver if things were passed as parameters. <br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +<br>
> +@dataclass(slots=True, frozen=True)<br>
> +class ExecutionConfiguration:<br>
> +    system_under_test: NodeConfiguration<br>
> +<br>
Minor comment: seems strange having only a single member variable in this<br>
class, effectively duplicating the class above.<br></blockquote><div><br></div><div>More is intended to go here. For instance, what tests to run, configuration for virtual machines, the traffic generator node.</div><div> </div><div><snip></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +    @staticmethod<br>
> +    def from_dict(d: dict, node_map: dict) -> "ExecutionConfiguration":<br>
from reading the code it appears that node_map is a dict of<br>
NodeConfiguration objects, right? Might be worth adding that to the<br>
definition for clarity, and also the specific type of the dict "d" (if it<br>
has one)<br>
> +        sut_name = d["system_under_test"]<br>
> +        assert sut_name in node_map, f"Unknown SUT {sut_name} in execution {d}"<br>
> +<br>
> +        return ExecutionConfiguration(<br>
> +            system_under_test=node_map[sut_name],<br>
> +        )<br>
> +<br>
> +<br>
> +@dataclass(slots=True, frozen=True)<br>
> +class Configuration:<br>
> +    executions: list[ExecutionConfiguration]<br>
> +<br>
> +    @staticmethod<br>
> +    def from_dict(d: dict) -> "Configuration":<br>
> +        nodes: list[NodeConfiguration] = list(<br>
> +            map(NodeConfiguration.from_dict, d["nodes"])<br>
So "d" is a dict of dicts?<br></blockquote><div><br></div><div>d is a dictionary which matches the json schema for the class. In the case of the Configuration class, it is a dictionary matching the entire json schema.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +        )<br>
> +        assert len(nodes) > 0, "There must be a node to test"<br>
> +<br>
> +        node_map = {<a href="http://node.name" rel="noreferrer" target="_blank">node.name</a>: node for node in nodes}<br>
> +        assert len(nodes) == len(node_map), "Duplicate node names are not allowed"<br>
> +<br>
> +        executions: list[ExecutionConfiguration] = list(<br>
> +            map(<br>
> +                ExecutionConfiguration.from_dict, d["executions"], [node_map for _ in d]<br>
> +            )<br>
> +        )<br>
> +<br>
> +        return Configuration(executions=executions)<br>
> +<br>
> +<br>
> +def load_config() -> Configuration:<br>
> +    """<br>
> +    Loads the configuration file and the configuration file schema,<br>
> +    validates the configuration file, and creates a configuration object.<br>
> +    """<br>
> +    with open(SETTINGS.config_file_path, "r") as f:<br>
> +        config_data = yaml.safe_load(f)<br>
> +<br>
> +    schema_path = os.path.join(<br>
> +        pathlib.Path(__file__).parent.resolve(), "conf_yaml_schema.json"<br>
> +    )<br>
> +<br>
> +    with open(schema_path, "r") as f:<br>
> +        schema = json.load(f)<br>
> +    config: dict[str, Any] = warlock.model_factory(schema, name="_Config")(config_data)<br>
> +    config_obj: Configuration = Configuration.from_dict(dict(config))<br>
> +    return config_obj<br>
> +<br>
> +<br>
> +CONFIGURATION = load_config()<br>
<snip></blockquote></div>