Improve config file mechanism

changelogs/fragments/1.2.0.yml

@@ -0,0 +1 @@
+release_summary: Feature release.

changelogs/fragments/7-config.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - "Make config file management more flexible to allow project-specific config file format extensions for the explicitly passed configuration files (https://github.com/ansible-community/antsibull-core/pull/7)."

pyproject.toml

@@ -8,7 +8,7 @@ build-backend = "poetry.core.masonry.api"
 
 [tool.poetry]
 name = "antsibull-core"
-version = "1.1.0.post0"
+version = "1.2.0"
 description = "Tools for building the Ansible Distribution"
 authors = ["Toshio Kuratomi <[email protected]>", "Felix Fontein <[email protected]>"]
 license = "GPL-3.0-or-later"

src/antsibull_core/app_context.py

@@ -53,7 +53,6 @@ def run(args):
 
 import argparse
 import contextvars
-import functools
 import sys
 import typing as t
 from contextlib import contextmanager
@@ -67,13 +66,8 @@ def run(args):
     import aiocontextvars  # noqa: F401, pylint:disable=unused-import
 
 
-#: Field names in the args and config whose value will be added to the app_ctx
-_FIELDS_IN_APP_CTX = frozenset(('ansible_base_url', 'breadcrumbs', 'galaxy_url', 'indexes',
-                                'logging_cfg', 'pypi_url', 'use_html_blobs', 'collection_cache'))
+AppContextT = t.TypeVar('AppContextT', bound=AppContext)
 
-#: Field names in the args and config whose value will be added to the lib_ctx
-_FIELDS_IN_LIB_CTX = frozenset(
-    ('chunksize', 'doc_parsing_backend', 'max_retries', 'process_max', 'thread_max'))
 
 #: lib_ctx should be restricted to things which do not belong in the API but an application or
 #: user might want to tweak.  Global, internal, incidental values are good to store here.  Things
@@ -109,7 +103,7 @@ def run(args):
 app_ctx: 'contextvars.ContextVar[AppContext]' = contextvars.ContextVar('app_ctx')
 
 
-class ContextReturn(t.NamedTuple):
+class ContextReturn(t.Generic[AppContextT]):
     """
     NamedTuple for the return value of :func:`create_contexts`.
 
@@ -124,11 +118,33 @@ class ContextReturn(t.NamedTuple):
     :ivar cfg: Configuration values which were not used to construct the contexts.
     """
 
-    app_ctx: AppContext
+    app_ctx: AppContextT
     lib_ctx: LibContext
     args: argparse.Namespace
     cfg: t.Dict
 
+    # pylint: disable-next=redefined-outer-name
+    def __init__(self, app_ctx: AppContextT, lib_ctx: LibContext,
+                 args: argparse.Namespace, cfg: t.Dict):
+        self.app_ctx = app_ctx
+        self.lib_ctx = lib_ctx
+        self.args = args
+        self.cfg = cfg
+
+    def __getitem__(self, index: int) -> t.Any:
+        if index == 0:
+            return self.app_ctx
+        if index == 1:
+            return self.lib_ctx
+        if index == 2:
+            return self.args
+        if index == 3:
+            return self.cfg
+        raise IndexError('tuple index out of range')
+
+    def __iter__(self) -> t.Iterable:
+        return (self.app_ctx, self.lib_ctx, self.args, self.cfg).__iter__()
+
 
 def _extract_context_values(known_fields, args: t.Optional[argparse.Namespace],
                             cfg: t.Mapping = ImmutableDict()) -> t.Dict:
@@ -152,13 +168,30 @@ def _extract_context_values(known_fields, args: t.Optional[argparse.Namespace],
     return context_values
 
 
-_extract_lib_context_values = functools.partial(_extract_context_values, _FIELDS_IN_LIB_CTX)
-_extract_app_context_values = functools.partial(_extract_context_values, _FIELDS_IN_APP_CTX)
+@t.overload
+def create_contexts(args: t.Optional[argparse.Namespace] = None,
+                    cfg: t.Mapping = ImmutableDict(),
+                    use_extra: bool = True,
+                    ) -> ContextReturn[AppContext]:
+    ...
+
+
+@t.overload
+def create_contexts(args: t.Optional[argparse.Namespace] = None,
+                    cfg: t.Mapping = ImmutableDict(),
+                    use_extra: bool = True,
+                    *,
+                    app_context_model: t.Type[AppContextT],
+                    ) -> ContextReturn[AppContextT]:
+    ...
 
 
 def create_contexts(args: t.Optional[argparse.Namespace] = None,
                     cfg: t.Mapping = ImmutableDict(),
-                    use_extra: bool = True) -> ContextReturn:
+                    use_extra: bool = True,
+                    *,
+                    app_context_model=AppContext,
+                    ) -> ContextReturn:
     """
     Create new contexts appropriate for setting the app and lib context.
 
@@ -172,7 +205,9 @@ def create_contexts(args: t.Optional[argparse.Namespace] = None,
     :kwarg use_extra: When True, the default, all extra arguments and config values will be set as
         fields in ``app_ctx.extra``.  When False, the extra arguments and config values will be
         returned as part of the ContextReturn.
-    :returns: A ContextReturn NamedTuple.
+    :kwarg app_context_model: The model to use for the app context. Must be derived from
+        :obj:`AppContext`. If not provided, will use :obj:`AppContext` itself.
+    :returns: A ``ContextReturn`` object.
 
     .. warning::
         We cannot tell whether a user set a value via the command line if :python:mod:`argparse`
@@ -185,13 +220,16 @@ def create_contexts(args: t.Optional[argparse.Namespace] = None,
         If the field is only used via the :attr:`AppContext.extra` mechanism (not explictly set),
         then you should ignore this section and use :python:mod:`argparse`'s default mechanism.
     """
-    lib_values = _extract_lib_context_values(args, cfg)
-    app_values = _extract_app_context_values(args, cfg)
+    fields_in_lib_ctx = set(LibContext.__fields__)
+    fields_in_app_ctx = set(app_context_model.__fields__)
+    known_fields = fields_in_app_ctx.union(fields_in_lib_ctx)
+
+    lib_values = _extract_context_values(fields_in_lib_ctx, args, cfg)
+    app_values = _extract_context_values(fields_in_app_ctx, args, cfg)
 
     #
     # Save the unused values
     #
-    known_fields = _FIELDS_IN_APP_CTX.union(_FIELDS_IN_LIB_CTX)
 
     unused_cfg = {}
     if cfg:
@@ -211,7 +249,7 @@ def create_contexts(args: t.Optional[argparse.Namespace] = None,
     unused_args_ns = argparse.Namespace(**unused_args)
 
     # create new app and lib ctxt from the application's arguments and config.
-    local_app_ctx = AppContext(**app_values)
+    local_app_ctx = app_context_model(**app_values)
     local_lib_ctx = LibContext(**lib_values)
 
     return ContextReturn(
@@ -255,13 +293,24 @@ def lib_context(new_context: t.Optional[LibContext] = None) -> t.Generator[LibCo
     lib_ctx.reset(reset_token)
 
 
+@t.overload
 @contextmanager
-def app_context(new_context: t.Optional[AppContext] = None) -> t.Generator[AppContext, None, None]:
+def app_context() -> t.ContextManager[AppContext]:
+    ...
+
+
+@t.overload
+@contextmanager
+def app_context(new_context: AppContextT) -> t.ContextManager[AppContextT]:
+    ...
+
+
+@contextmanager
+def app_context(new_context=None):
     """
     Set up a new app_context.
 
-    :kwarg new_context: New app context to setup.  If this is None, the context is set to a copy of
-        the old context.
+    :kwarg new_context: New app context to setup.
     """
     if new_context is None:
         new_context = _copy_app_context()
@@ -273,8 +322,8 @@ def app_context(new_context: t.Optional[AppContext] = None) -> t.Generator[AppCo
 
 
 @contextmanager
-def app_and_lib_context(context_data: ContextReturn
-                        ) -> t.Generator[t.Tuple[AppContext, LibContext], None, None]:
+def app_and_lib_context(context_data: ContextReturn[AppContextT]
+                        ) -> t.Generator[t.Tuple[AppContextT, LibContext], None, None]:
     """
     Set the app and lib context at the same time.
 
@@ -293,6 +342,19 @@ def app_and_lib_context(context_data: ContextReturn
             yield (new_app_ctx, new_lib_ctx)
 
 
+class AppContextWrapper(t.Generic[AppContextT]):
+    def __repr__(self):
+        return "<ContextVarWrapper name='app_ctx'>"
+
+    @property
+    def name(self):
+        return 'app_ctx'
+
+    @staticmethod
+    def get() -> AppContextT:
+        return t.cast(AppContextT, app_ctx.get())
+
+
 #
 # Set initial contexts with default values
 #

src/antsibull_core/config.py

@@ -5,14 +5,15 @@
 # SPDX-FileCopyrightText: 2020, Ansible Project
 """Functions to handle config files."""
 
-import itertools
 import os.path
 import typing as t
 
 import perky  # type: ignore[import]
+import pydantic as p
 
 from .logging import log
 from .schemas.config import ConfigModel
+from .schemas.context import AppContext, LibContext
 
 
 mlog = log.fields(mod=__name__)
@@ -24,6 +25,10 @@
 USER_CONFIG_FILE = '~/.antsibull.cfg'
 
 
+class ConfigError(Exception):
+    pass
+
+
 def find_config_files(conf_files: t.Iterable[str]) -> t.List[str]:
     """
     Find all config files that exist.
@@ -70,7 +75,49 @@ def read_config(filename: str) -> ConfigModel:
     return raw_config_data
 
 
-def load_config(conf_files: t.Union[t.Iterable[str], str, None] = None) -> t.Dict:
+def validate_config(config: t.Mapping, filenames: t.List[str],
+                    app_context_model: t.Type[AppContext]) -> None:
+    """
+    Validate configuration.
+
+    Given the configuration loaded from one or more configuration files and the app context model,
+    splits up the config into lib context and app context part and validates both parts with the
+    given model. Raises a :obj:`ConfigError` if validation fails.
+    """
+    lib_fields = set(LibContext.__fields__)
+    lib = {}
+    app = {}
+    for key, value in config.items():
+        if key in lib_fields:
+            lib[key] = value
+        else:
+            app[key] = value
+    # Note: We parse the object but discard the model because we want to validate the config but let
+    # the context handle all setting of defaults
+    try:
+        LibContext.parse_obj(lib)
+        app_context_model.parse_obj(app)
+    except p.ValidationError as exc:
+        raise ConfigError(
+            f"Error while parsing configuration from {', '.join(filenames)}:\n{exc}") from exc
+
+
+def _load_config_file(filename: str) -> t.Mapping:
+    """
+    Load configuration from one file and return the raw data.
+    """
+    try:
+        return perky.load(filename)
+    except OSError as exc:
+        raise ConfigError(
+            f"Error while loading configuration from {filename}: {exc}") from exc
+    except perky.PerkyFormatError as exc:
+        raise ConfigError(
+            f"Error while parsing configuration from {filename}:\n{exc}") from exc
+
+
+def load_config(conf_files: t.Union[t.Iterable[str], str, None] = None,
+                app_context_model: t.Type[AppContext] = AppContext) -> t.Dict:
     """
     Load configuration.
 
@@ -90,29 +137,26 @@ def load_config(conf_files: t.Union[t.Iterable[str], str, None] = None) -> t.Dic
     elif conf_files is None:
         conf_files = ()
 
-    user_config_file = os.path.expanduser(USER_CONFIG_FILE)
-    available_files = find_config_files(itertools.chain((SYSTEM_CONFIG_FILE, user_config_file),
-                                                        conf_files))
+    implicit_files = find_config_files((SYSTEM_CONFIG_FILE, os.path.expanduser(USER_CONFIG_FILE)))
+    explicit_files = find_config_files(conf_files)
 
-    includes = list(available_files)
+    flog.fields(implicit_files=implicit_files,
+                explicit_files=explicit_files).debug('found config files')
 
-    flog.debug('loading config files')
-    # Perky has some bugs that prevent this simple way from working:
-    # https://github.com/ansible-community/antsibull/pull/118
-    # cfg = {'includes': includes}
-    # cfg = perky.includes(cfg, recursive=True)
+    flog.debug('loading implicit config files')
+    cfg: t.Dict = {}
+    for filename in implicit_files:
+        cfg.update(_load_config_file(filename))
 
-    # Workaround for above bug.  Note that includes specified in the config files will not work
-    # but we can just add that as a new feature when perky gets it working.
-    cfg = {}
-    for filename in includes:
-        new_cfg = perky.load(filename)
-        cfg.update(new_cfg)
+    flog.debug('validating implicit configuration')
+    validate_config(cfg, implicit_files, AppContext)
 
-    flog.debug('validating configuration')
-    # Note: We parse the object but discard the model because we want to validate the config but let
-    # the context handle all setting of defaults
-    ConfigModel.parse_obj(cfg)
+    flog.debug('loading explicit config files')
+    for filename in explicit_files:
+        cfg.update(_load_config_file(filename))
+
+    flog.debug('validating combined configuration')
+    validate_config(cfg, implicit_files + explicit_files, app_context_model)
 
     flog.fields(config=cfg).debug('Leave')
     return cfg

src/antsibull_core/schemas/config.py

@@ -116,6 +116,7 @@ class LoggingModel(BaseModel):
      })
 
 
+# This class is no longer needed, it will eventually be removed
 class ConfigModel(BaseModel):
     # pyre-ignore[8]: https://github.com/samuelcolvin/pydantic/issues/1684
     ansible_base_url: p.HttpUrl = 'https://github.com/ansible/ansible'  # type: ignore[assignment]

src/antsibull_core/schemas/context.py

@@ -73,10 +73,10 @@ class AppContext(BaseModel):
     use_html_blobs: p.StrictBool = False
     collection_cache: t.Optional[str] = None
 
-    _convert_bools = p.validator('breadcrumbs', 'indexes', 'use_html_blobs',
-                                 pre=True, allow_reuse=True)(convert_bool)
-    _convert_paths = p.validator('collection_cache',
-                                 pre=True, allow_reuse=True)(convert_path)
+    _antsibull_core_convert_bools = p.validator('breadcrumbs', 'indexes', 'use_html_blobs',
+                                                pre=True, allow_reuse=True)(convert_bool)
+    _antsibull_core_convert_paths = p.validator('collection_cache',
+                                                pre=True, allow_reuse=True)(convert_path)
 
 
 class LibContext(BaseModel):