Realized why ${} interpolation wasn't turned on

src/bandersnatch/config/core.py

@@ -1,3 +1,5 @@
+import importlib.resources
+import shutil
 import sys
 from collections.abc import Mapping
 from configparser import ConfigParser, ExtendedInterpolation
@@ -44,3 +46,18 @@ def get_validated(self, model: type[_C]) -> _C:
         if name not in self._validate_config_models:
             self._validate_config_models[name] = model.from_config_parser(self)
         return cast(_C, self._validate_config_models[name])
+
+    @classmethod
+    def from_path(
+        cls, source_file: Path, *, defaults: Mapping[str, str] | None = None
+    ) -> "BandersnatchConfig":
+        config = cls(defaults=defaults)
+        config.read_path(source_file)
+        return config
+
+
+def write_default_config_file(dest: Path) -> None:
+    with importlib.resources.path(
+        "bandersnatch", "default.conf"
+    ) as default_config_path:
+        shutil.copy(default_config_path, dest)

src/bandersnatch/config/mirror_options.py

@@ -136,6 +136,25 @@ def from_config_parser(cls, source: ConfigParser) -> "MirrorOptions":
 
         return instance
 
+    @classmethod
+    def populate_defaults(cls, config: ConfigParser) -> None:
+        if not config.has_section("mirror"):
+            config.add_section("mirror")
+        section = config["mirror"]
+
+        option: attrs.Attribute
+        for option in attrs.fields(cls):
+            if option.default is attrs.NOTHING or option.default is None:
+                continue
+            option_name = option.alias or option.name
+            if isinstance(option.default, PurePath):
+                option_value = option.default.as_posix()
+            elif not isinstance(option.default, str):
+                option_value = str(option.default)
+            else:
+                option_value = option.default
+            section[option_name] = option_value
+
 
 def _check_legacy_reference(config: ConfigParser, value: str) -> str | None:
     if not has_legacy_config_ref(value):

src/bandersnatch/main.py

@@ -4,16 +4,17 @@
 import logging.config
 import shutil
 import sys
-from configparser import ConfigParser
 from pathlib import Path
 from tempfile import gettempdir
 
-import bandersnatch.configuration
 import bandersnatch.delete
 import bandersnatch.log
 import bandersnatch.master
 import bandersnatch.mirror
 import bandersnatch.verify
+from bandersnatch.config import BandersnatchConfig
+from bandersnatch.config.core import write_default_config_file
+from bandersnatch.config.errors import ConfigurationError
 from bandersnatch.storage import storage_backend_plugins
 
 # See if we have uvloop and use if so
@@ -149,7 +150,7 @@ def _make_parser() -> argparse.ArgumentParser:
     return parser
 
 
-async def async_main(args: argparse.Namespace, config: ConfigParser) -> int:
+async def async_main(args: argparse.Namespace, config: BandersnatchConfig) -> int:
     if args.op.lower() == "delete":
         async with bandersnatch.master.Master(
             config.get("mirror", "master"),
@@ -203,22 +204,23 @@ def main(loop: asyncio.AbstractEventLoop | None = None) -> int:
     # Prepare default config file if needed.
     config_path = Path(args.config)
     if not config_path.exists():
-        logger.warning(f"Config file '{args.config}' missing, creating default config.")
+        logger.warning(f"Config file '{args.config}' missing; creating default config.")
         logger.warning("Please review the config file, then run 'bandersnatch' again.")
-
-        default_config_path = Path(__file__).parent / "default.conf"
         try:
-            shutil.copy(default_config_path, args.config)
+            write_default_config_file(config_path)
         except OSError as e:
-            logger.error(f"Could not create config file: {e}")
+            logger.error("Could not create config file: %s", e)
         return 1
 
-    config = bandersnatch.configuration.BandersnatchConfig(
-        config_file=args.config
-    ).config
+    try:
+        config = BandersnatchConfig.from_path(config_path)
+    except (ConfigurationError, OSError) as err:
+        logger.error("Error reading config file: %s", str(err))
+        logger.debug("Error reading config file; exception info:", exc_info=err)
+        return 1
 
     if config.has_option("mirror", "log-config"):
-        logging.config.fileConfig(str(Path(config.get("mirror", "log-config"))))
+        logging.config.fileConfig(Path(config.get("mirror", "log-config")))
 
     if loop:
         loop.set_debug(args.debug)

src/bandersnatch/mirror.py

@@ -1,5 +1,4 @@
 import asyncio
-import configparser
 import datetime
 import hashlib
 import logging
@@ -8,20 +7,23 @@
 import time
 from collections.abc import Awaitable, Callable
 from json import dump
-from pathlib import Path, WindowsPath
+from pathlib import Path, PurePath
 from threading import RLock
 from typing import Any, TypeVar
 from urllib.parse import unquote, urlparse
 
 from filelock import Timeout
 
+from bandersnatch.config.errors import ConfigurationError
+
 from . import utils
-from .configuration import validate_config_values
+from .config import BandersnatchConfig, MirrorOptions
+from .config.comparison_method import ComparisonMethod
 from .errors import PackageNotFound
 from .filter import LoadedFilters
 from .master import Master
 from .package import Package
-from .simple import SimpleAPI, SimpleFormat, SimpleFormats
+from .simple import SimpleAPI, SimpleDigest, SimpleFormat, SimpleFormats
 from .storage import Storage, storage_backend_plugins
 
 LOG_PLUGINS = True
@@ -178,39 +180,36 @@ class BandersnatchMirror(Mirror):
 
     def __init__(
         self,
-        homedir: Path,
+        homedir: PurePath,
         master: Master,
         storage_backend: Storage,
         filters: LoadedFilters,
         stop_on_error: bool = False,
         workers: int = 3,
         hash_index: bool = False,
         json_save: bool = False,
-        digest_name: str | None = None,
-        root_uri: str | None = None,
+        digest_name: SimpleDigest = SimpleDigest.SHA256,
+        root_uri: str = "",
         keep_index_versions: int = 0,
         diff_append_epoch: bool = False,
-        diff_full_path: Path | str | None = None,
+        diff_full_path: Path | None = None,
         flock_timeout: int = 1,
         diff_file_list: list[Path] | None = None,
         *,
         cleanup: bool = False,
         release_files_save: bool = True,
-        compare_method: str | None = None,
+        compare_method: ComparisonMethod = ComparisonMethod.HASH,
         download_mirror: str | None = None,
         download_mirror_no_fallback: bool | None = False,
-        simple_format: SimpleFormat | str = "ALL",
+        simple_format: SimpleFormat = SimpleFormat.ALL,
     ) -> None:
         super().__init__(master=master, filters=filters, workers=workers)
         self.cleanup = cleanup
 
         self.storage_backend = storage_backend
         self.stop_on_error = stop_on_error
         self.loop = asyncio.get_event_loop()
-        if isinstance(homedir, WindowsPath):
-            self.homedir = self.storage_backend.PATH_BACKEND(homedir.as_posix())
-        else:
-            self.homedir = self.storage_backend.PATH_BACKEND(str(homedir))
+        self.homedir = self.storage_backend.PATH_BACKEND(homedir.as_posix())
         self.lockfile_path = self.homedir / ".lock"
         self.master = master
 
@@ -226,12 +225,12 @@ def __init__(
         # This is generally not necessary, but was added for the official internal
         # PyPI mirror, which requires serving packages from
         # https://files.pythonhosted.org
-        self.root_uri = root_uri or ""
+        self.root_uri = root_uri
         self.diff_append_epoch = diff_append_epoch
         self.diff_full_path = diff_full_path
         self.keep_index_versions = keep_index_versions
-        self.digest_name = digest_name if digest_name else "sha256"
-        self.compare_method = compare_method if compare_method else "hash"
+        self.digest_name = digest_name
+        self.compare_method = compare_method
         self.download_mirror = download_mirror
         self.download_mirror_no_fallback = download_mirror_no_fallback
         self.workers = workers
@@ -921,7 +920,7 @@ async def _run_in_storage_executor(
 
 
 async def _setup_diff_file(
-    storage_plugin: Storage, configured_path: str, append_epoch: bool
+    storage_plugin: Storage, configured_path: PurePath, append_epoch: bool
 ) -> Path:
     # use the storage backend to convert an abstract path to a concrete one
     concrete_path = storage_plugin.PATH_BACKEND(configured_path)
@@ -948,61 +947,63 @@ async def _setup_diff_file(
 
 
 async def mirror(
-    config: configparser.ConfigParser,
+    config: BandersnatchConfig,
     specific_packages: list[str] | None = None,
     sync_simple_index: bool = True,
 ) -> int:
-    config_values = validate_config_values(config)
+    try:
+        mirror_options = config.get_validated(MirrorOptions)
+    except ConfigurationError as err:
+        logger.error("Configuration error: %s", str(err))
+        logger.debug("Configuration error: exception info:", exc_info=err)
+        return 1
 
     storage_plugin = next(
         iter(
             storage_backend_plugins(
                 config=config,
-                backend=config_values.storage_backend_name,
+                backend=mirror_options.storage_backend_name,
                 clear_cache=True,
             )
         )
     )
 
     diff_full_path: Path | None = None
-    if config_values.diff_file_path:
+    if mirror_options.diff_file:
         diff_full_path = await _setup_diff_file(
             storage_plugin,
-            config_values.diff_file_path,
-            config_values.diff_append_epoch,
+            mirror_options.diff_file,
+            mirror_options.diff_append_epoch,
         )
 
-    mirror_url = config.get("mirror", "master")
-    timeout = config.getfloat("mirror", "timeout")
-    global_timeout = config.getfloat("mirror", "global-timeout", fallback=None)
-    proxy = config.get("mirror", "proxy", fallback=None)
-    homedir = Path(config.get("mirror", "directory"))
-
     # Always reference those classes here with the fully qualified name to
     # allow them being patched by mock libraries!
-    async with Master(mirror_url, timeout, global_timeout, proxy) as master:
+    async with Master(
+        mirror_options.master_url,
+        mirror_options.timeout,
+        mirror_options.global_timeout,
+        mirror_options.proxy_url,
+    ) as master:
         mirror = BandersnatchMirror(
-            homedir,
+            mirror_options.directory,
             master,
             storage_plugin,
             LoadedFilters(config, load_all=True),
-            stop_on_error=config.getboolean("mirror", "stop-on-error"),
-            workers=config.getint("mirror", "workers"),
-            hash_index=config.getboolean("mirror", "hash-index"),
-            json_save=config_values.json_save,
-            root_uri=config_values.root_uri,
-            digest_name=config_values.digest_name,
-            compare_method=config_values.compare_method,
-            keep_index_versions=config.getint(
-                "mirror", "keep_index_versions", fallback=0
-            ),
-            diff_append_epoch=config_values.diff_append_epoch,
+            stop_on_error=mirror_options.stop_on_error,
+            workers=mirror_options.workers,
+            hash_index=mirror_options.hash_index,
+            json_save=mirror_options.save_json,
+            root_uri=mirror_options.root_uri,
+            digest_name=mirror_options.digest_name,
+            compare_method=mirror_options.compare_method,
+            keep_index_versions=mirror_options.keep_index_versions,
+            diff_append_epoch=mirror_options.diff_append_epoch,
             diff_full_path=diff_full_path,
-            cleanup=config_values.cleanup,
-            release_files_save=config_values.release_files_save,
-            download_mirror=config_values.download_mirror,
-            download_mirror_no_fallback=config_values.download_mirror_no_fallback,
-            simple_format=config_values.simple_format,
+            cleanup=mirror_options.cleanup,
+            release_files_save=mirror_options.save_release_files,
+            download_mirror=mirror_options.download_mirror_url,
+            download_mirror_no_fallback=mirror_options.download_mirror_no_fallback,
+            simple_format=mirror_options.simple_format,
         )
         changed_packages = await mirror.synchronize(
             specific_packages, sync_simple_index=sync_simple_index

src/bandersnatch/tests/mock_config.py

@@ -1,15 +1,12 @@
-from bandersnatch.configuration import BandersnatchConfig
+from bandersnatch.config import BandersnatchConfig
 
 
 def mock_config(contents: str, filename: str = "test.conf") -> BandersnatchConfig:
     """
     Creates a config file with contents and loads them into a
     BandersnatchConfig instance.
     """
-    with open(filename, "w") as fd:
-        fd.write(contents)
 
     instance = BandersnatchConfig()
-    instance.config_file = filename
-    instance.load_configuration()
+    instance.read_string(contents)
     return instance