Skip to content
Snippets Groups Projects
Commit 2663c0a4 authored by vlorentz's avatar vlorentz
Browse files

config: Remove confusing magic in path handling

1. When passed a .yaml path (or any path with a non-whitelisted extension),
   don't read .yml instead when the .yaml exists.
   It's an extremely surprising behavior.

2. If the .yaml file does not exist, it will still try alternative extensions
   in order not to break existing deployments which may rely on it, but it
   raises a warning now.

3. When given a non-existing path, show an error log, but keep parsing
   it as an empty config, in order not to break existing deployments.
parent 906769ae
No related branches found
Tags v2.22.1
1 merge request!352config: Remove confusing magic in path handling
Pipeline #2427 passed
...@@ -70,34 +70,35 @@ def exists_accessible(filepath: str) -> bool: ...@@ -70,34 +70,35 @@ def exists_accessible(filepath: str) -> bool:
raise PermissionError("Permission denied: {filepath!r}") raise PermissionError("Permission denied: {filepath!r}")
def config_basepath(config_path: str) -> str:
"""Return the base path of a configuration file"""
if config_path.endswith(".yml"):
return config_path[:-4]
return config_path
def read_raw_config(base_config_path: str) -> Dict[str, Any]: def read_raw_config(base_config_path: str) -> Dict[str, Any]:
"""Read the raw config corresponding to base_config_path. """Read the raw config corresponding to base_config_path.
Can read yml files. Can read yml files.
""" """
yml_file = f"{base_config_path}.yml" yml_file = config_path(base_config_path)
if exists_accessible(yml_file): if yml_file is None:
logging.error("Config file %s does not exist, ignoring it.", base_config_path)
return {}
else:
logger.debug("Loading config file %s", yml_file) logger.debug("Loading config file %s", yml_file)
with open(yml_file) as f: with open(yml_file) as f:
return yaml.safe_load(f) return yaml.safe_load(f)
return {}
def config_path(config_path):
def config_exists(config_path):
"""Check whether the given config exists""" """Check whether the given config exists"""
basepath = config_basepath(config_path) if exists_accessible(config_path):
return any( return config_path
exists_accessible(basepath + extension) for extension in SWH_CONFIG_EXTENSIONS for extension in SWH_CONFIG_EXTENSIONS:
) if exists_accessible(config_path + extension):
logger.warning(
"%s does not exist, using %s instead",
config_path,
config_path + extension,
)
return config_path + extension
return None
def read( def read(
...@@ -122,7 +123,7 @@ def read( ...@@ -122,7 +123,7 @@ def read(
conf: Dict[str, Any] = {} conf: Dict[str, Any] = {}
if conf_file: if conf_file:
base_config_path = config_basepath(os.path.expanduser(conf_file)) base_config_path = os.path.expanduser(conf_file)
conf = read_raw_config(base_config_path) or {} conf = read_raw_config(base_config_path) or {}
if not default_conf: if not default_conf:
...@@ -152,8 +153,8 @@ def priority_read( ...@@ -152,8 +153,8 @@ def priority_read(
# Try all the files in order # Try all the files in order
for filename in conf_filenames: for filename in conf_filenames:
full_filename = os.path.expanduser(filename) full_filename = config_path(os.path.expanduser(filename))
if config_exists(full_filename): if full_filename is not None:
return read(full_filename, default_conf) return read(full_filename, default_conf)
# Else, return the default configuration # Else, return the default configuration
...@@ -303,6 +304,6 @@ def load_from_envvar(default_config: Optional[Dict[str, Any]] = None) -> Dict[st ...@@ -303,6 +304,6 @@ def load_from_envvar(default_config: Optional[Dict[str, Any]] = None) -> Dict[st
), "SWH_CONFIG_FILENAME environment variable is undefined." ), "SWH_CONFIG_FILENAME environment variable is undefined."
cfg_path = os.environ["SWH_CONFIG_FILENAME"] cfg_path = os.environ["SWH_CONFIG_FILENAME"]
cfg = read_raw_config(config_basepath(cfg_path)) cfg = read_raw_config(cfg_path)
cfg = merge_configs(default_config or {}, cfg) cfg = merge_configs(default_config or {}, cfg)
return cfg return cfg
# Copyright (C) 2015-2020 The Software Heritage developers # Copyright (C) 2015-2023 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version # License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
import os import os
from pathlib import Path
import shutil import shutil
import pkg_resources.extern.packaging.version import pkg_resources.extern.packaging.version
...@@ -115,6 +116,16 @@ def test_read(swh_config): ...@@ -115,6 +116,16 @@ def test_read(swh_config):
assert res == parsed_conffile assert res == parsed_conffile
def test_read_dotyaml(swh_config):
swh_config2 = Path(str(swh_config).replace(".yml", ".yaml"))
swh_config.rename(swh_config2)
# when
res = config.read(swh_config2, default_conf)
# then
assert res == parsed_conffile
def test_read_no_default_conf(swh_config): def test_read_no_default_conf(swh_config):
"""If no default config if provided to read, this should directly parse the config file """If no default config if provided to read, this should directly parse the config file
yaml yaml
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment