Consolidate logging configuration method and tests

#3

The `configure_logging()` method was previously located in `start.py`,
but the logging configuration dictionary was in `logging_conf.py`. This
organization was not ideal for readability and separation of concerns.
It also created logistical complications, such as requiring the Gunicorn
configuration file `gunicorn_conf.py` to import from start.py, so that
it could configure logging for Gunicorn. The `start.py` script should be
self-contained, and should import other modules and objects from the
package, not the other way around.

This commit will move `configure_logging()` to `logging_conf.py`, and
move the tests to a corresponding module, `test_logging_conf.py`. This
matches up nicely with the latest updates to the `gunicorn_conf.py`, by
having a setup method at the top of the module, and the settings below.

inboard/gunicorn_conf.py

@@ -2,7 +2,7 @@
 import os
 from typing import Optional
 
-from inboard.start import configure_logging
+from inboard.logging_conf import configure_logging
 
 
 def calculate_workers(

inboard/logging_conf.py

@@ -1,5 +1,39 @@
+import importlib.util
+import logging
+import logging.config
 import os
 import sys
+from pathlib import Path
+
+
+def configure_logging(
+    logger: logging.Logger = logging.getLogger(),
+    logging_conf: str = os.getenv("LOGGING_CONF", "inboard.logging_conf"),
+) -> dict:
+    """Configure Python logging based on a path to a logging module or file."""
+    try:
+        logging_conf_path = Path(logging_conf)
+        spec = (
+            importlib.util.spec_from_file_location("confspec", logging_conf_path)
+            if logging_conf_path.is_file() and logging_conf_path.suffix == ".py"
+            else importlib.util.find_spec(logging_conf)
+        )
+        if not spec:
+            raise ImportError(f"Unable to import {logging_conf}")
+        logging_conf_module = importlib.util.module_from_spec(spec)
+        spec.loader.exec_module(logging_conf_module)  # type: ignore[union-attr]
+        if not hasattr(logging_conf_module, "LOGGING_CONFIG"):
+            raise AttributeError(f"No LOGGING_CONFIG in {logging_conf_module.__name__}")
+        logging_conf_dict = getattr(logging_conf_module, "LOGGING_CONFIG")
+        if not isinstance(logging_conf_dict, dict):
+            raise TypeError("LOGGING_CONFIG is not a dictionary instance")
+        logging.config.dictConfig(logging_conf_dict)
+        logger.debug(f"Logging dict config loaded from {logging_conf_path}.")
+        return logging_conf_dict
+    except Exception as e:
+        logger.error(f"Error when setting logging module: {e.__class__.__name__} {e}.")
+        raise
+
 
 LOG_COLORS = (
     True

inboard/start.py

@@ -1,42 +1,14 @@
 #!/usr/bin/env python3
 import importlib.util
 import logging
-import logging.config
 import os
 import subprocess
 from pathlib import Path
 from typing import Optional
 
 import uvicorn  # type: ignore
 
-
-def configure_logging(
-    logger: logging.Logger = logging.getLogger(),
-    logging_conf: str = os.getenv("LOGGING_CONF", "inboard.logging_conf"),
-) -> dict:
-    """Configure Python logging based on a path to a logging module or file."""
-    try:
-        logging_conf_path = Path(logging_conf)
-        spec = (
-            importlib.util.spec_from_file_location("confspec", logging_conf_path)
-            if logging_conf_path.is_file() and logging_conf_path.suffix == ".py"
-            else importlib.util.find_spec(logging_conf)
-        )
-        if not spec:
-            raise ImportError(f"Unable to import {logging_conf}")
-        logging_conf_module = importlib.util.module_from_spec(spec)
-        spec.loader.exec_module(logging_conf_module)  # type: ignore[union-attr]
-        if not hasattr(logging_conf_module, "LOGGING_CONFIG"):
-            raise AttributeError(f"No LOGGING_CONFIG in {logging_conf_module.__name__}")
-        logging_conf_dict = getattr(logging_conf_module, "LOGGING_CONFIG")
-        if not isinstance(logging_conf_dict, dict):
-            raise TypeError("LOGGING_CONFIG is not a dictionary instance")
-        logging.config.dictConfig(logging_conf_dict)
-        logger.debug(f"Logging dict config loaded from {logging_conf_path}.")
-        return logging_conf_dict
-    except Exception as e:
-        logger.error(f"Error when setting logging module: {e.__class__.__name__} {e}.")
-        raise
+from inboard.logging_conf import configure_logging
 
 
 def run_pre_start_script(logger: logging.Logger = logging.getLogger()) -> str:

tests/test_logging_conf.py

@@ -0,0 +1,148 @@
+import os
+from pathlib import Path
+
+import pytest
+from pytest_mock import MockerFixture
+
+from inboard import logging_conf
+
+
+class TestConfigureLogging:
+    """Test logging configuration method.
+    ---
+    """
+
+    def test_configure_logging_file(
+        self, logging_conf_file_path: Path, mocker: MockerFixture
+    ) -> None:
+        """Test logging configuration with correct logging config file path."""
+        mock_logger = mocker.patch.object(logging_conf.logging, "root", autospec=True)
+        logging_conf.configure_logging(
+            logger=mock_logger, logging_conf=str(logging_conf_file_path)
+        )
+        mock_logger.debug.assert_called_once_with(
+            f"Logging dict config loaded from {logging_conf_file_path}."
+        )
+
+    def test_configure_logging_module(
+        self, logging_conf_module_path: str, mocker: MockerFixture
+    ) -> None:
+        """Test logging configuration with correct logging config module path."""
+        mock_logger = mocker.patch.object(logging_conf.logging, "root", autospec=True)
+        logging_conf.configure_logging(
+            logger=mock_logger, logging_conf=logging_conf_module_path
+        )
+        mock_logger.debug.assert_called_once_with(
+            f"Logging dict config loaded from {logging_conf_module_path}."
+        )
+
+    def test_configure_logging_module_incorrect(self, mocker: MockerFixture) -> None:
+        """Test logging configuration with incorrect logging config module path."""
+        mock_logger = mocker.patch.object(logging_conf.logging, "root", autospec=True)
+        mock_logger_error_msg = "Error when setting logging module"
+        with pytest.raises(ModuleNotFoundError):
+            logging_conf.configure_logging(
+                logger=mock_logger, logging_conf="no.module.here"
+            )
+        assert mock_logger_error_msg in mock_logger.error.call_args.args[0]
+        assert "ModuleNotFoundError" in mock_logger.error.call_args.args[0]
+
+    def test_configure_logging_tmp_file(
+        self, logging_conf_tmp_file_path: Path, mocker: MockerFixture
+    ) -> None:
+        """Test logging configuration with temporary logging config file path."""
+        mock_logger = mocker.patch.object(logging_conf.logging, "root", autospec=True)
+        logging_conf_file = f"{logging_conf_tmp_file_path}/tmp_log.py"
+        logging_conf.configure_logging(
+            logger=mock_logger, logging_conf=logging_conf_file
+        )
+        mock_logger.debug.assert_called_once_with(
+            f"Logging dict config loaded from {logging_conf_file}."
+        )
+
+    def test_configure_logging_tmp_file_incorrect_extension(
+        self,
+        logging_conf_tmp_path_incorrect_extension: Path,
+        mocker: MockerFixture,
+    ) -> None:
+        """Test logging configuration with incorrect temporary file type."""
+        mock_logger = mocker.patch.object(logging_conf.logging, "root", autospec=True)
+        incorrect_logging_conf = logging_conf_tmp_path_incorrect_extension.joinpath(
+            "tmp_logging_conf"
+        )
+        logger_error_msg = "Error when setting logging module"
+        import_error_msg = f"Unable to import {incorrect_logging_conf}"
+        with pytest.raises(ImportError) as e:
+            logging_conf.configure_logging(
+                logger=mock_logger,
+                logging_conf=str(incorrect_logging_conf),
+            )
+        assert str(e.value) in import_error_msg
+        mock_logger.error.assert_called_once_with(
+            f"{logger_error_msg}: ImportError {import_error_msg}."
+        )
+        with open(incorrect_logging_conf, "r") as f:
+            contents = f.read()
+            assert "This file doesn't have the correct extension" in contents
+
+    def test_configure_logging_tmp_module(
+        self,
+        logging_conf_tmp_file_path: Path,
+        mocker: MockerFixture,
+        monkeypatch: pytest.MonkeyPatch,
+    ) -> None:
+        """Test logging configuration with temporary logging config path."""
+        mock_logger = mocker.patch.object(logging_conf.logging, "root", autospec=True)
+        monkeypatch.syspath_prepend(logging_conf_tmp_file_path)
+        monkeypatch.setenv("LOGGING_CONF", "tmp_log")
+        assert os.getenv("LOGGING_CONF") == "tmp_log"
+        logging_conf.configure_logging(logger=mock_logger, logging_conf="tmp_log")
+        mock_logger.debug.assert_called_once_with(
+            "Logging dict config loaded from tmp_log."
+        )
+
+    def test_configure_logging_tmp_module_incorrect_type(
+        self,
+        logging_conf_tmp_path_incorrect_type: Path,
+        mocker: MockerFixture,
+        monkeypatch: pytest.MonkeyPatch,
+    ) -> None:
+        """Test logging configuration with temporary logging config path.
+        - Correct module name
+        - `LOGGING_CONFIG` object with incorrect type
+        """
+        mock_logger = mocker.patch.object(logging_conf.logging, "root", autospec=True)
+        monkeypatch.syspath_prepend(logging_conf_tmp_path_incorrect_type)
+        monkeypatch.setenv("LOGGING_CONF", "incorrect_type")
+        logger_error_msg = "Error when setting logging module"
+        type_error_msg = "LOGGING_CONFIG is not a dictionary instance"
+        assert os.getenv("LOGGING_CONF") == "incorrect_type"
+        with pytest.raises(TypeError):
+            logging_conf.configure_logging(
+                logger=mock_logger, logging_conf="incorrect_type"
+            )
+        mock_logger.error.assert_called_once_with(
+            f"{logger_error_msg}: TypeError {type_error_msg}."
+        )
+
+    def test_configure_logging_tmp_module_no_dict(
+        self,
+        logging_conf_tmp_path_no_dict: Path,
+        mocker: MockerFixture,
+        monkeypatch: pytest.MonkeyPatch,
+    ) -> None:
+        """Test logging configuration with temporary logging config path.
+        - Correct module name
+        - No `LOGGING_CONFIG` object
+        """
+        mock_logger = mocker.patch.object(logging_conf.logging, "root", autospec=True)
+        monkeypatch.syspath_prepend(logging_conf_tmp_path_no_dict)
+        monkeypatch.setenv("LOGGING_CONF", "no_dict")
+        logger_error_msg = "Error when setting logging module"
+        attribute_error_msg = "No LOGGING_CONFIG in no_dict"
+        assert os.getenv("LOGGING_CONF") == "no_dict"
+        with pytest.raises(AttributeError):
+            logging_conf.configure_logging(logger=mock_logger, logging_conf="no_dict")
+        mock_logger.error.assert_called_once_with(
+            f"{logger_error_msg}: AttributeError {attribute_error_msg}."
+        )

tests/test_start.py

@@ -7,141 +7,6 @@
 from inboard import start
 
 
-class TestConfigureLogging:
-    """Test logging configuration methods.
-    ---
-    """
-
-    def test_configure_logging_file(
-        self, logging_conf_file_path: Path, mocker: MockerFixture
-    ) -> None:
-        """Test `start.configure_logging` with correct logging config file path."""
-        mock_logger = mocker.patch.object(start.logging, "root", autospec=True)
-        start.configure_logging(
-            logger=mock_logger, logging_conf=str(logging_conf_file_path)
-        )
-        mock_logger.debug.assert_called_once_with(
-            f"Logging dict config loaded from {logging_conf_file_path}."
-        )
-
-    def test_configure_logging_module(
-        self, logging_conf_module_path: str, mocker: MockerFixture
-    ) -> None:
-        """Test `start.configure_logging` with correct logging config module path."""
-        mock_logger = mocker.patch.object(start.logging, "root", autospec=True)
-        start.configure_logging(
-            logger=mock_logger, logging_conf=logging_conf_module_path
-        )
-        mock_logger.debug.assert_called_once_with(
-            f"Logging dict config loaded from {logging_conf_module_path}."
-        )
-
-    def test_configure_logging_module_incorrect(self, mocker: MockerFixture) -> None:
-        """Test `start.configure_logging` with incorrect logging config module path."""
-        mock_logger = mocker.patch.object(start.logging, "root", autospec=True)
-        mock_logger_error_msg = "Error when setting logging module"
-        with pytest.raises(ModuleNotFoundError):
-            start.configure_logging(logger=mock_logger, logging_conf="no.module.here")
-        assert mock_logger_error_msg in mock_logger.error.call_args.args[0]
-        assert "ModuleNotFoundError" in mock_logger.error.call_args.args[0]
-
-    def test_configure_logging_tmp_file(
-        self, logging_conf_tmp_file_path: Path, mocker: MockerFixture
-    ) -> None:
-        """Test `start.configure_logging` with temporary logging config file path."""
-        mock_logger = mocker.patch.object(start.logging, "root", autospec=True)
-        logging_conf_file = f"{logging_conf_tmp_file_path}/tmp_log.py"
-        start.configure_logging(logger=mock_logger, logging_conf=logging_conf_file)
-        mock_logger.debug.assert_called_once_with(
-            f"Logging dict config loaded from {logging_conf_file}."
-        )
-
-    def test_configure_logging_tmp_file_incorrect_extension(
-        self,
-        logging_conf_tmp_path_incorrect_extension: Path,
-        mocker: MockerFixture,
-    ) -> None:
-        """Test `start.configure_logging` with incorrect temporary file type."""
-        mock_logger = mocker.patch.object(start.logging, "root", autospec=True)
-        incorrect_logging_conf = logging_conf_tmp_path_incorrect_extension.joinpath(
-            "tmp_logging_conf"
-        )
-        logger_error_msg = "Error when setting logging module"
-        import_error_msg = f"Unable to import {incorrect_logging_conf}"
-        with pytest.raises(ImportError) as e:
-            start.configure_logging(
-                logger=mock_logger,
-                logging_conf=str(incorrect_logging_conf),
-            )
-        assert str(e.value) in import_error_msg
-        mock_logger.error.assert_called_once_with(
-            f"{logger_error_msg}: ImportError {import_error_msg}."
-        )
-        with open(incorrect_logging_conf, "r") as f:
-            contents = f.read()
-            assert "This file doesn't have the correct extension" in contents
-
-    def test_configure_logging_tmp_module(
-        self,
-        logging_conf_tmp_file_path: Path,
-        mocker: MockerFixture,
-        monkeypatch: pytest.MonkeyPatch,
-    ) -> None:
-        """Test `start.configure_logging` with temporary logging config path."""
-        mock_logger = mocker.patch.object(start.logging, "root", autospec=True)
-        monkeypatch.syspath_prepend(logging_conf_tmp_file_path)
-        monkeypatch.setenv("LOGGING_CONF", "tmp_log")
-        assert os.getenv("LOGGING_CONF") == "tmp_log"
-        start.configure_logging(logger=mock_logger, logging_conf="tmp_log")
-        mock_logger.debug.assert_called_once_with(
-            "Logging dict config loaded from tmp_log."
-        )
-
-    def test_configure_logging_tmp_module_incorrect_type(
-        self,
-        logging_conf_tmp_path_incorrect_type: Path,
-        mocker: MockerFixture,
-        monkeypatch: pytest.MonkeyPatch,
-    ) -> None:
-        """Test `start.configure_logging` with temporary logging config path.
-        - Correct module name
-        - `LOGGING_CONFIG` object with incorrect type
-        """
-        mock_logger = mocker.patch.object(start.logging, "root", autospec=True)
-        monkeypatch.syspath_prepend(logging_conf_tmp_path_incorrect_type)
-        monkeypatch.setenv("LOGGING_CONF", "incorrect_type")
-        logger_error_msg = "Error when setting logging module"
-        type_error_msg = "LOGGING_CONFIG is not a dictionary instance"
-        assert os.getenv("LOGGING_CONF") == "incorrect_type"
-        with pytest.raises(TypeError):
-            start.configure_logging(logger=mock_logger, logging_conf="incorrect_type")
-        mock_logger.error.assert_called_once_with(
-            f"{logger_error_msg}: TypeError {type_error_msg}."
-        )
-
-    def test_configure_logging_tmp_module_no_dict(
-        self,
-        logging_conf_tmp_path_no_dict: Path,
-        mocker: MockerFixture,
-        monkeypatch: pytest.MonkeyPatch,
-    ) -> None:
-        """Test `start.configure_logging` with temporary logging config path.
-        - Correct module name
-        - No `LOGGING_CONFIG` object
-        """
-        mock_logger = mocker.patch.object(start.logging, "root", autospec=True)
-        monkeypatch.syspath_prepend(logging_conf_tmp_path_no_dict)
-        monkeypatch.setenv("LOGGING_CONF", "no_dict")
-        logger_error_msg = "Error when setting logging module"
-        attribute_error_msg = "No LOGGING_CONFIG in no_dict"
-        assert os.getenv("LOGGING_CONF") == "no_dict"
-        with pytest.raises(AttributeError):
-            start.configure_logging(logger=mock_logger, logging_conf="no_dict")
-        mock_logger.error.assert_called_once_with(
-            f"{logger_error_msg}: AttributeError {attribute_error_msg}."
-        )
-
-
 class TestRunPreStartScript:
     """Run pre-start scripts using the method in `start.py`.
     ---