diff --git a/swh/indexer/storage/api/server.py b/swh/indexer/storage/api/server.py --- a/swh/indexer/storage/api/server.py +++ b/swh/indexer/storage/api/server.py @@ -1,10 +1,11 @@ -# Copyright (C) 2015-2019 The Software Heritage developers +# Copyright (C) 2015-2020 The Software Heritage developers # 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 import logging import os +from typing import Any, Dict, Optional from swh.core import config from swh.core.api import RPCServerApp @@ -54,14 +55,16 @@ api_cfg = None -def load_and_check_config(config_file, type="local"): +def load_and_check_config( + config_path: Optional[str], type: str = "local" +) -> Dict[str, Any]: """Check the minimal configuration is set to run the api or raise an error explanation. Args: - config_file (str): Path to the configuration file to load - type (str): configuration type. For 'local' type, more - checks are done. + config_path: Path to the configuration file to load + type: configuration type. For 'local' type, more + checks are done. Raises: Error if the setup is not as expected @@ -70,13 +73,13 @@ configuration as a dict """ - if not config_file: + if not config_path: raise EnvironmentError("Configuration file must be defined") - if not os.path.exists(config_file): - raise FileNotFoundError("Configuration file %s does not exist" % (config_file,)) + if not os.path.exists(config_path): + raise FileNotFoundError(f"Configuration file {config_path} does not exist") - cfg = config.read(config_file) + cfg = config.read(config_path) if "indexer_storage" not in cfg: raise KeyError("Missing '%indexer_storage' configuration") @@ -106,8 +109,8 @@ """ global api_cfg if not api_cfg: - config_file = os.environ.get("SWH_CONFIG_FILENAME") - api_cfg = load_and_check_config(config_file) + config_path = os.environ.get("SWH_CONFIG_FILENAME") + api_cfg = load_and_check_config(config_path) app.config.update(api_cfg) handler = logging.StreamHandler() app.logger.addHandler(handler) diff --git a/swh/indexer/tests/storage/test_server.py b/swh/indexer/tests/storage/test_server.py --- a/swh/indexer/tests/storage/test_server.py +++ b/swh/indexer/tests/storage/test_server.py @@ -31,53 +31,49 @@ return str(config_path) -def test_load_and_check_config_no_configuration() -> None: +@pytest.mark.parametrize("config_path", [None, ""]) +def test_load_and_check_config_no_configuration(config_path) -> None: """Inexistent configuration files raises""" - with pytest.raises(EnvironmentError) as e: - load_and_check_config(None) + with pytest.raises(EnvironmentError, match="Configuration file must be defined"): + load_and_check_config(config_path) - assert e.value.args[0] == "Configuration file must be defined" +def test_load_and_check_inexistent_config_path() -> None: config_path = "/indexer/inexistent/config.yml" - with pytest.raises(FileNotFoundError) as e: + expected_error = f"Configuration file {config_path} does not exist" + with pytest.raises(FileNotFoundError, match=expected_error): load_and_check_config(config_path) - assert e.value.args[0] == "Configuration file %s does not exist" % (config_path,) - def test_load_and_check_config_wrong_configuration(tmpdir) -> None: """Wrong configuration raises""" config_path = prepare_config_file(tmpdir, "something: useless") - with pytest.raises(KeyError) as e: + with pytest.raises(KeyError, match="Missing '%indexer_storage' configuration"): load_and_check_config(config_path) - assert e.value.args[0] == "Missing '%indexer_storage' configuration" - def test_load_and_check_config_remote_config_local_type_raise(tmpdir) -> None: """'local' configuration without 'local' storage raises""" config = {"indexer_storage": {"cls": "remote", "args": {}}} config_path = prepare_config_file(tmpdir, config) - with pytest.raises(ValueError) as e: - load_and_check_config(config_path, type="local") - assert ( - e.value.args[0] - == "The indexer_storage backend can only be started with a 'local' " + expected_error = ( + "The indexer_storage backend can only be started with a 'local' " "configuration" ) + with pytest.raises(ValueError, match=expected_error): + load_and_check_config(config_path, type="local") def test_load_and_check_config_local_incomplete_configuration(tmpdir) -> None: """Incomplete 'local' configuration should raise""" config = {"indexer_storage": {"cls": "local", "args": {}}} + expected_error = "Invalid configuration; missing 'db' config entry" config_path = prepare_config_file(tmpdir, config) - with pytest.raises(ValueError) as e: + with pytest.raises(ValueError, match=expected_error): load_and_check_config(config_path) - assert e.value.args[0] == "Invalid configuration; missing 'db' config entry" - def test_load_and_check_config_local_config_fine(tmpdir) -> None: """'Remote configuration is fine"""