Merge pull request #100 from praekeltfoundation/log-config-problems

Log all config validation errors as warnings, including (ignored) extra keys

Author: jerith

CHANGELOG.md

@@ -5,8 +5,9 @@
 - Message cache for storing inbound messages
 - Link last inbound message to outbound messages for USSD flows
 - Retry messages if Turn rate limits us
+- Warning logs for config errors, including (ignored) extra keys
 
-## [0.3.2] - 2024-01-22
+## [0.3.2] - 2025-01-22
 ### Added
 - Add Turn Channels application
 

src/vumi2/config.py

@@ -1,6 +1,7 @@
 import os
 from argparse import Namespace
 from collections.abc import Callable, Iterable
+from logging import getLogger
 from pathlib import Path
 from typing import (
     Any,
@@ -13,7 +14,9 @@
 import yaml
 from attrs import Attribute, AttrsInstance, Factory, define, fields
 from attrs import has as is_attrs
-from cattrs import Converter
+from cattrs import Converter, transform_error
+
+logger = getLogger(__name__)
 
 _conv = Converter(prefer_attrib_converters=True)
 
@@ -29,6 +32,11 @@ def get_config_class(cls: Configurable[CT] | type[Configurable[CT]]) -> type[CT]
 
 
 def structure(config: dict, cls: type[CT]) -> CT:
+    try:
+        _conv.copy(forbid_extra_keys=True).structure(config, cls)
+    except Exception as e:
+        for err in transform_error(e, cls.__name__):
+            logger.warning(f"Config error: {err}")
     return _conv.structure(config, cls)
 
 

tests/test_config.py

@@ -1,8 +1,10 @@
+import logging
 import os
 from argparse import Namespace
 from pathlib import Path
 
 import pytest
+from cattrs import ClassValidationError
 
 from vumi2.config import (
     BaseConfig,
@@ -18,6 +20,9 @@ def deserialise(config: dict) -> BaseConfig:
 
 
 def test_default_base_config():
+    """
+    All fields in BaseConfig have default values.
+    """
     config = deserialise({})
     assert config.amqp.hostname == "127.0.0.1"
     assert config.amqp.port == 5672
@@ -29,6 +34,9 @@ def test_default_base_config():
 
 
 def test_specified_base_config():
+    """
+    Specified values override all defaults.
+    """
     config = deserialise(
         {
             "amqp": {
@@ -52,6 +60,9 @@ def test_specified_base_config():
 
 
 def test_load_config_from_environment():
+    """
+    Config values can be provided in environment variables.
+    """
     environment = {
         "VUMI_AMQP_HOSTNAME": "localhost",
         "VUMI_AMQP_PORT": "1234",
@@ -79,6 +90,9 @@ def test_load_config_from_environment():
 
 
 def test_load_config_from_cli():
+    """
+    Config values can be provided as cli args.
+    """
     # Make sure we don't have a stray config file set in the environment.
     assert "VUMI_CONFIG_FILE" not in os.environ
 
@@ -179,3 +193,50 @@ def test_missing_config_file(monkeypatch, tmp_path):
 
     with pytest.raises(FileNotFoundError):
         load_config(cli=Namespace())
+
+
+def test_extra_field_warnings_logged(caplog):
+    """
+    Extra fields are logged as warnings, but don't fail validation.
+    """
+    config = deserialise(
+        {
+            "amqp": {
+                "vhost": "/vumi",
+                "not_amqp": "blah",
+            },
+            "worker_concurrency": 10,
+            "extra": "value",
+        }
+    )
+    assert config.amqp.vhost == "/vumi"
+    assert config.worker_concurrency == 10
+
+    logs = [log.message for log in caplog.records if log.levelno >= logging.WARNING]
+    assert logs == [
+        "Config error: extra fields found (not_amqp) @ BaseConfig.amqp",
+        "Config error: extra fields found (extra) @ BaseConfig",
+    ]
+
+
+def test_config_validation_warnings_logged(caplog):
+    """
+    All validation failures are logged as warnings.
+    """
+    with pytest.raises(ClassValidationError):
+        deserialise(
+            {
+                "amqp": {
+                    "vhost": "/vumi",
+                    "port": "eleventy one",
+                },
+                "worker_concurrency": 10,
+                "extra": "value",
+            }
+        )
+
+    logs = [log.message for log in caplog.records if log.levelno >= logging.WARNING]
+    assert logs == [
+        "Config error: invalid value for type, expected int @ BaseConfig.amqp.port",
+        "Config error: extra fields found (extra) @ BaseConfig",
+    ]