Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable overriding values in goth config file #489

Merged
merged 3 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 40 additions & 7 deletions goth/configuration.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
"""Defines a class representing `goth` configuration."""
from pathlib import Path
from typing import Any, Dict, List, Optional, Type, Union
from typing import Any, Dict, List, Optional, Tuple, Type, Union

import dpath.util
import yaml

from goth.address import YAGNA_REST_URL, PROXY_HOST
Expand All @@ -15,6 +16,14 @@
from goth.runner.probe import Probe, YagnaContainerConfig


Override = Tuple[str, Any]
"""Type representing a single value override in a YAML config file.

First element is a path within the file, e.g.: `"docker-compose.build-environment"`.
Second element is the value to be inserted under the given path.
"""


class Configuration:
"""Configuration of a `goth` test network."""

Expand Down Expand Up @@ -83,8 +92,8 @@ class ConfigurationParseError(Exception):
class _ConfigurationParser:
"""A class for reading `goth` configuration from a `dict` instance."""

# A type for documents parsed by configuration parser
Doc = Union[Dict[str, Any], List[Any]]
"""Type for documents parsed by _ConfigurationParser."""

def __init__(self, doc: Doc, config_path: Optional[Path], root_key: str = ""):
self._doc: _ConfigurationParser.Doc = doc
Expand All @@ -107,7 +116,6 @@ def __getitem__(self, key: Union[int, str]) -> Any:
raise ConfigurationParseError(f"Required key is missing: {child_key}")

def __iter__(self):

if isinstance(self._doc, dict):
return iter(self._doc)

Expand Down Expand Up @@ -165,10 +173,12 @@ def read_compose_config(self) -> ComposeConfig:
self.ensure_type(dict)

docker_dir = self.get_path("docker-dir")
assert docker_dir

log_patterns = self.get("compose-log-patterns")
log_patterns.ensure_type(dict)

build_env_config = self["build-environment"]
build_env_config = self.get("build-environment")
# `build_env_config` may be `None` if no optional build parameters
# (binary path, commit hash etc.) are specified in the config file
if build_env_config:
Expand Down Expand Up @@ -200,13 +210,23 @@ def read_build_env(self, docker_dir: Path) -> YagnaBuildEnvironment:
)


def load_yaml(yaml_path: Union[Path, str]) -> Configuration:
"""Load a configuration from a YAML file at `yaml_path'."""
def load_yaml(
yaml_path: Union[Path, str], overrides: Optional[List[Override]] = None
) -> Configuration:
"""Load a configuration from a YAML file at `yaml_path'.

It's possible to override values from the YAML file through the use of `overrides`.
Each override is a tuple of a dict path and a value to insert at that path.
Dict paths are dot-separated flattened paths in the YAML file, e.g.:
`"docker-compose.build-environment.binary-path"`.
"""

import importlib

with open(str(yaml_path)) as f:
dict_ = yaml.load(f, yaml.FullLoader)
dict_: Dict[str, Any] = yaml.load(f, yaml.FullLoader)
if overrides:
_apply_overrides(dict_, overrides)

network = _ConfigurationParser(dict_, Path(yaml_path))

Expand Down Expand Up @@ -252,3 +272,16 @@ def load_yaml(yaml_path: Union[Path, str]) -> Configuration:
)

return config


def _apply_overrides(dict_: Dict[str, Any], overrides: List[Override]):
for (dict_path, value) in overrides:
path_list: List[str] = dict_path.split(".")
Copy link
Contributor

@azawlocki azawlocki Apr 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could also allow ints in path_list and then override like this:

    overrides = [("nodes.1.name", "dostawca-1")]  # path_list = ["nodes", 1, "name"]
    config = load_yaml(default_config_file, overrides)
    assert config.containers[1].name == "dostawca-1"

But it's probably not worth doing, if we ever need to identify config elements based on list indices then it would be probably an indication that the relevant part of the configuration tree should be made a dir instead of a list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to limit ourselves to dicts for now. The only use-cases I see for list access in our current config file is for debugging purposes. As such, this can be achieved by editing the file itself, rather than using overrides.
With that said I'm going to mention the limitation to dict in the docstring for load_yaml.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 8f83b54


leaf = dpath.util.get(dict_, path_list, default=None)
# if the path's last element does not exist, add it as a new dict
if not leaf:
leaf_name = path_list.pop()
value = {leaf_name: value}

dpath.util.new(dict_, path_list, value)
81 changes: 46 additions & 35 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ aiohttp = "3.7.3"
ansicolors = "^1.1.0"
docker-compose = "^1.29"
docker = "^5.0"
dpath = "^2.0"
func_timeout = "4.3.5"
mitmproxy = "^5.3"
pyyaml = "^5.4"
Expand Down
41 changes: 41 additions & 0 deletions test/goth/test_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,44 @@ def test_parse_default_config(default_config_file: Path):

config = load_yaml(default_config_file)
assert config.compose_config.build_env


def test_load_yaml_override_existing(default_config_file: Path):
"""Test overriding an existing field in a YAML config file."""

test_key = "zksync"
test_value = ".*I am overridden!.*"
overrides = [
(f"docker-compose.compose-log-patterns.{test_key}", test_value),
]

config = load_yaml(default_config_file, overrides)

assert config.compose_config.log_patterns[test_key] == test_value


def test_load_yaml_override_new(default_config_file: Path):
"""Test adding a new field to a YAML config file through overrides."""

test_value = "v0.0.1-rc666"
Copy link
Contributor

@azawlocki azawlocki Apr 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤘

overrides = [
("docker-compose.build-environment.release-tag", test_value),
]

config = load_yaml(default_config_file, overrides)

assert config.compose_config.build_env.release_tag == test_value


def test_load_yaml_override_top_level(default_config_file: Path):
"""Test overriding a value under a top-level dict in the YAML file."""

test_value = "overridden-name"
overrides = [
("web-root", test_value),
]

config = load_yaml(default_config_file, overrides)

assert config.web_root
assert config.web_root.name == test_value