diff --git a/swh/core/config.py b/swh/core/config.py --- a/swh/core/config.py +++ b/swh/core/config.py @@ -9,7 +9,7 @@ from itertools import chain from copy import deepcopy -from typing import Any, Callable, Dict, Optional, Tuple +from typing import Any, Callable, Dict, List, Optional, Tuple logger = logging.getLogger(__name__) @@ -47,7 +47,7 @@ } -def exists_accessible(file): +def exists_accessible(filepath: str) -> bool: """Check whether a file exists, and is accessible. Returns: @@ -59,16 +59,16 @@ """ try: - os.stat(file) + os.stat(filepath) except PermissionError: raise except FileNotFoundError: return False else: - if os.access(file, os.R_OK): + if os.access(filepath, os.R_OK): return True else: - raise PermissionError("Permission denied: %r" % file) + raise PermissionError("Permission denied: {filepath!r}") def config_basepath(config_path: str) -> str: @@ -142,7 +142,9 @@ return conf -def priority_read(conf_filenames, default_conf=None): +def priority_read( + conf_filenames: List[str], default_conf: Optional[Dict[str, Tuple[str, Any]]] = None +): """Try reading the configuration files from conf_filenames, in order, and return the configuration from the first one that exists. @@ -169,7 +171,7 @@ return full_config -def merge_configs(base, other): +def merge_configs(base: Optional[Dict[str, Any]], other: Optional[Dict[str, Any]]): """Merge two config dictionaries This does merge config dicts recursively, with the rules, for every value @@ -240,7 +242,7 @@ return output -def swh_config_paths(base_filename): +def swh_config_paths(base_filename: str) -> List[str]: """Return the Software Heritage specific configuration paths for the given filename.""" @@ -286,68 +288,34 @@ class SWHConfig: - """Mixin to add configuration parsing abilities to classes + """Mixin to add swh configuration parsing abilities to classes. - The class should override the class attributes: - - DEFAULT_CONFIG (default configuration to be parsed) - - CONFIG_BASE_FILENAME (the filename of the configuration to be used) + The class should override the class attributes DEFAULT_CONFIG (default typed + configuration, same specification as defined in the `fn`:read function.) - This class defines one classmethod, parse_config_file, which - parses a configuration file using the default config as set in the - class attribute. + This class defines one classmethod, `fn`:parse_config_file, which parses a + configuration file, provided through the SWH_CONFIG_FILENAME environment variable. + + Eventually enriched with default configuration """ - DEFAULT_CONFIG = {} # type: Dict[str, Tuple[str, Any]] - CONFIG_BASE_FILENAME = "" # type: Optional[str] + DEFAULT_CONFIG: Dict[str, Tuple[str, Any]] = {} @classmethod - def parse_config_file( - cls, - base_filename=None, - config_filename=None, - additional_configs=None, - global_config=True, - ): + def parse_config_file(cls): """Parse the configuration file associated to the current class. - By default, parse_config_file will load the configuration - cls.CONFIG_BASE_FILENAME from one of the Software Heritage - configuration directories, in order, unless it is overridden - by base_filename or config_filename (which shortcuts the file - lookup completely). - - Args: - - base_filename (str): overrides the default - cls.CONFIG_BASE_FILENAME - - config_filename (str): sets the file to parse instead of - the defaults set from cls.CONFIG_BASE_FILENAME - - additional_configs: (list of default configuration dicts) - allows to override or extend the configuration set in - cls.DEFAULT_CONFIG. - - global_config (bool): Load the global configuration (default: - True) - """ - - if config_filename: - config_filenames = [config_filename] - elif "SWH_CONFIG_FILENAME" in os.environ: - config_filenames = [os.environ["SWH_CONFIG_FILENAME"]] - else: - if not base_filename: - base_filename = cls.CONFIG_BASE_FILENAME - config_filenames = swh_config_paths(base_filename) - if not additional_configs: - additional_configs = [] - - full_default_config = merge_default_configs( - cls.DEFAULT_CONFIG, *additional_configs - ) + By default, parse_config_file will load the configuration from the file + referenced by the environment variable SWH_CONFIG_FILENAME. If not set, this + will raise a value error requesting the environment variable to be set. - config = {} - if global_config: - config = load_global_config() + """ - config.update(priority_read(config_filenames, full_default_config)) + if "SWH_CONFIG_FILENAME" not in os.environ: + raise ValueError( + "SWH_CONFIG_FILENAME environment variable should be defined." + ) - return config + config_path = os.environ["SWH_CONFIG_FILENAME"] + return read(config_path, cls.DEFAULT_CONFIG) diff --git a/swh/core/tests/test_config.py b/swh/core/tests/test_config.py --- a/swh/core/tests/test_config.py +++ b/swh/core/tests/test_config.py @@ -326,3 +326,44 @@ config.merge_configs({"a": v}, {"a": {}}) with pytest.raises(TypeError): config.merge_configs({"a": {}}, {"a": v}) + + +def test_swhconfig_mixin_no_swh_config_filename(): + """Without SWH_CONFIG_FILENAME set, SWHConfig.parse_config_file raises""" + + with pytest.raises(ValueError, match="SWH_CONFIG_FILENAME environment"): + config.SWHConfig.parse_config_file() + + +def test_swhconfig_mixin_no_default(swh_config, monkeypatch): + config_path = str(swh_config) + monkeypatch.setenv("SWH_CONFIG_FILENAME", config_path) + + actual_config = config.SWHConfig.parse_config_file() + + expected_config = config.read(config_path) + assert actual_config == expected_config + + +def test_swhconfig_mixin_with_inheritance(swh_config, monkeypatch): + class TestConfig(config.SWHConfig): + """Inherited class with SWHConfig mixin enhanced. + + """ + + DEFAULT_CONFIG = { + "number": ("int", 666), + "something-cool": ("list[str]", "something,cool"), + } + + config_path = str(swh_config) + monkeypatch.setenv("SWH_CONFIG_FILENAME", config_path) + + actual_config = TestConfig.parse_config_file() + + expected_config = config.read(config_path) + expected_config.update( + {"number": 666, "something-cool": "something,cool",} + ) + + assert actual_config == expected_config