diff --git a/swh/vault/api/server.py b/swh/vault/api/server.py --- a/swh/vault/api/server.py +++ b/swh/vault/api/server.py @@ -3,6 +3,8 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information +from __future__ import annotations + import asyncio import os from typing import Any, Dict, Optional @@ -49,7 +51,7 @@ def check_config(cfg: Dict[str, Any]) -> Dict[str, Any]: - """Ensure the configuration is ok to run a local vault server + """Ensure the configuration is ok to run a local vault server, and propagate defaults. Raises: EnvironmentError if the configuration is not for local instance @@ -57,9 +59,11 @@ scheduler Returns: - Configuration dict to instantiate a local vault server instance + New configuration dict to instantiate a local vault server instance. """ + cfg = cfg.copy() + if "vault" not in cfg: raise ValueError("missing 'vault' configuration") @@ -68,31 +72,35 @@ raise EnvironmentError( "The vault backend can only be started with a 'local' configuration", ) - args = vcfg["args"] - if "cache" not in args: - args["cache"] = cfg.get("cache") - if "storage" not in args: - args["storage"] = cfg.get("storage") - if "scheduler" not in args: - args["scheduler"] = cfg.get("scheduler") + + # TODO: Soft-deprecation of args key. Remove when ready. + vcfg.update(vcfg.get("args", {})) + + # Default to top-level value if any + if "cache" not in vcfg: + vcfg["cache"] = cfg.get("cache") + if "storage" not in vcfg: + vcfg["storage"] = cfg.get("storage") + if "scheduler" not in vcfg: + vcfg["scheduler"] = cfg.get("scheduler") for key in ("cache", "storage", "scheduler"): - if not args.get(key): + if not vcfg.get(key): raise ValueError(f"invalid configuration: missing {key} config entry.") return cfg -def make_app(config_to_check: Dict[str, Any]) -> VaultServerApp: +def make_app(config: Dict[str, Any]) -> VaultServerApp: """Ensure the configuration is ok, then instantiate the server application """ - config_ok = check_config(config_to_check) + config = check_config(config) app = VaultServerApp( __name__, backend_class=VaultInterface, - backend_factory=lambda: get_vault(config_ok["vault"]), - client_max_size=config_ok["client_max_size"], + backend_factory=lambda: get_vault(config["vault"]), + client_max_size=config["client_max_size"], ) app.router.add_route("GET", "/", index) return app diff --git a/swh/vault/cookers/__init__.py b/swh/vault/cookers/__init__.py --- a/swh/vault/cookers/__init__.py +++ b/swh/vault/cookers/__init__.py @@ -2,7 +2,11 @@ # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information + +from __future__ import annotations + import os +from typing import Any, Dict from swh.core.config import load_named_config from swh.core.config import read as read_config @@ -24,6 +28,41 @@ return COOKER_TYPES[obj_type] +def check_config(cfg: Dict[str, Any]) -> Dict[str, Any]: + """Ensure the configuration is ok to run a vault worker, and propagate defaults + + Raises: + EnvironmentError if the configuration is not for remote instance + ValueError if one of the following keys is missing: vault, storage + + Returns: + New configuration dict to instantiate a vault worker instance + + """ + cfg = cfg.copy() + + if "vault" not in cfg: + raise ValueError("missing 'vault' configuration") + + vcfg = cfg["vault"] + if vcfg["cls"] != "remote": + raise EnvironmentError( + "This vault backend can only be a 'remote' configuration" + ) + + # TODO: Soft-deprecation of args key. Remove when ready. + vcfg.update(vcfg.get("args", {})) + + # Default to top-level value if any + if "storage" not in vcfg: + vcfg["storage"] = cfg.get("storage") + + if not vcfg.get("storage"): + raise ValueError("invalid configuration: missing 'storage' config entry.") + + return cfg + + def get_cooker(obj_type: str, obj_id: str): """Instantiate a cooker class of type obj_type. @@ -41,22 +80,11 @@ else: cfg = load_named_config(DEFAULT_CONFIG_PATH, DEFAULT_CONFIG) cooker_cls = get_cooker_cls(obj_type) - if "vault" not in cfg: - raise ValueError("missing 'vault' configuration") + cfg = check_config(cfg) vcfg = cfg["vault"] - if vcfg["cls"] != "remote": - raise EnvironmentError( - "This vault backend can only be a 'remote' configuration" - ) - args = vcfg["args"] - if "storage" not in args: - args["storage"] = cfg.get("storage") - - if not args.get("storage"): - raise ValueError("invalid configuration: missing 'storage' config entry.") - storage = get_storage(**args.pop("storage")) + storage = get_storage(**vcfg.pop("storage")) backend = get_vault(**vcfg) return cooker_cls( diff --git a/swh/vault/tests/conftest.py b/swh/vault/tests/conftest.py --- a/swh/vault/tests/conftest.py +++ b/swh/vault/tests/conftest.py @@ -73,7 +73,7 @@ @pytest.fixture def swh_local_vault_config(swh_vault_config: Dict[str, Any]) -> Dict[str, Any]: return { - "vault": {"cls": "local", "args": swh_vault_config}, + "vault": {"cls": "local", **swh_vault_config}, "client_max_size": 1024 ** 3, } diff --git a/swh/vault/tests/test_init_cookers.py b/swh/vault/tests/test_init_cookers.py --- a/swh/vault/tests/test_init_cookers.py +++ b/swh/vault/tests/test_init_cookers.py @@ -63,7 +63,6 @@ ValueError, "invalid configuration: missing 'storage' config entry", ), - ({"vault": {"cls": "remote"}}, KeyError, "args",), ], ) def test_get_cooker_config_ko( @@ -94,6 +93,10 @@ "vault": {"cls": "remote", "args": {"url": "mock://vault-backend",},}, "storage": {"cls": "remote", "url": "mock://storage-url"}, }, + { + "vault": {"cls": "remote", "url": "mock://vault-backend",}, + "storage": {"cls": "remote", "url": "mock://storage-url"}, + }, ], ) def test_get_cooker_nominal(config_ok, tmp_path, monkeypatch): diff --git a/swh/vault/tests/test_server.py b/swh/vault/tests/test_server.py --- a/swh/vault/tests/test_server.py +++ b/swh/vault/tests/test_server.py @@ -145,9 +145,9 @@ @pytest.mark.parametrize("missing_key", ["storage", "cache", "scheduler"]) def test_check_config_missing_key(missing_key, swh_vault_config) -> None: """Any other configuration than 'local' (the default) is rejected""" - config_ok = {"vault": {"cls": "local", "args": swh_vault_config}} + config_ok = {"vault": {"cls": "local", **swh_vault_config}} config_ko = copy.deepcopy(config_ok) - config_ko["vault"]["args"].pop(missing_key, None) + config_ko["vault"].pop(missing_key, None) expected_error = f"invalid configuration: missing {missing_key} config entry" with pytest.raises(ValueError, match=expected_error): @@ -157,5 +157,5 @@ @pytest.mark.parametrize("missing_key", ["storage", "cache", "scheduler"]) def test_check_config_ok(missing_key, swh_vault_config) -> None: """Any other configuration than 'local' (the default) is rejected""" - config_ok = {"vault": {"cls": "local", "args": swh_vault_config}} + config_ok = {"vault": {"cls": "local", **swh_vault_config}} assert check_config(config_ok) is not None