Refactor logging configuration

esmvalcore/_config.py

@@ -1,9 +1,7 @@
 """ESMValTool configuration."""
 import datetime
 import logging
-import logging.config
 import os
-import time
 from pathlib import Path
 
 import yaml
@@ -146,45 +144,6 @@ def load_config_developer(cfg_file=None):
     read_cmor_tables(CFG)
 
 
-def configure_logging(cfg_file=None, output_dir=None, console_log_level=None):
-    """Set up logging."""
-    if cfg_file is None:
-        cfg_file = os.path.join(os.path.dirname(__file__),
-                                'config-logging.yml')
-
-    cfg_file = os.path.abspath(cfg_file)
-    with open(cfg_file) as file_handler:
-        cfg = yaml.safe_load(file_handler)
-
-    if output_dir is None:
-        cfg['handlers'] = {
-            name: handler
-            for name, handler in cfg['handlers'].items()
-            if 'filename' not in handler
-        }
-        prev_root = cfg['root']['handlers']
-        cfg['root']['handlers'] = [
-            name for name in prev_root if name in cfg['handlers']
-        ]
-
-    log_files = []
-    for handler in cfg['handlers'].values():
-        if 'filename' in handler:
-            if not os.path.isabs(handler['filename']):
-                handler['filename'] = os.path.join(output_dir,
-                                                   handler['filename'])
-            log_files.append(handler['filename'])
-        if console_log_level is not None and 'stream' in handler:
-            if handler['stream'] in ('ext://sys.stdout', 'ext://sys.stderr'):
-                handler['level'] = console_log_level.upper()
-
-    logging.config.dictConfig(cfg)
-    logging.Formatter.converter = time.gmtime
-    logging.captureWarnings(True)
-
-    return log_files
-
-
 def get_project_config(project):
     """Get developer-configuration for project."""
     logger.debug("Retrieving %s configuration", project)

esmvalcore/_logging.py

@@ -0,0 +1,93 @@
+"""Configure logging."""
+
+import logging
+import logging.config
+import os
+import time
+from pathlib import Path
+
+import yaml
+
+
+def _purge_file_handlers(cfg: dict) -> None:
+    """Remove handlers with filename set.
+
+    This is used to remove file handlers which require an output
+    directory to be set.
+    """
+    cfg['handlers'] = {
+        name: handler
+        for name, handler in cfg['handlers'].items()
+        if 'filename' not in handler
+    }
+    prev_root = cfg['root']['handlers']
+    cfg['root']['handlers'] = [
+        name for name in prev_root if name in cfg['handlers']
+    ]
+
+
+def _get_log_files(cfg: dict, output_dir: str = None) -> list:
+    """Initialize log files for the file handlers."""
+    log_files = []
+
+    handlers = cfg['handlers']
+
+    for handler in handlers.values():
+        filename = handler.get('filename', None)
+
+        if filename:
+            if not os.path.isabs(filename):
+                handler['filename'] = os.path.join(output_dir, filename)
+            log_files.append(handler['filename'])
+
+    return log_files
+
+
+def _update_stream_level(cfg: dict, level=None):
+    """Update the log level for the stream handlers."""
+    handlers = cfg['handlers']
+
+    for handler in handlers.values():
+        if level is not None and 'stream' in handler:
+            if handler['stream'] in ('ext://sys.stdout', 'ext://sys.stderr'):
+                handler['level'] = level.upper()
+
+
+def configure_logging(cfg_file: str = None,
+                      output_dir: str = None,
+                      console_log_level: str = None) -> list:
+    """Configure logging.
+
+    Parameters
+    ----------
+    cfg_file : str, optional
+        Logging config file. If `None`, defaults to `configure-logging.yml`
+    output_dir : str, optional
+        Output directory for the log files. If `None`, log only to the console.
+    console_log_level : str, optional
+        If `None`, use the default (INFO).
+
+    Returns
+    -------
+    log_files : list
+        Filenames that will be logged to.
+    """
+    if cfg_file is None:
+        cfg_file = Path(__file__).parent / 'config-logging.yml'
+
+    cfg_file = Path(cfg_file).absolute()
+
+    with open(cfg_file) as file_handler:
+        cfg = yaml.safe_load(file_handler)
+
+    if output_dir is None:
+        _purge_file_handlers(cfg)
+
+    log_files = _get_log_files(cfg, output_dir=output_dir)
+    _update_stream_level(cfg, level=console_log_level)
+
+    logging.config.dictConfig(cfg)
+    logging.Formatter.converter = time.gmtime
+    logging.captureWarnings(True)
+
+    return log_files

esmvalcore/_main.py

@@ -118,7 +118,7 @@ def _copy_config_file(filename, overwrite, path):
         import os
         import shutil
 
-        from ._config import configure_logging
+        from ._logging import configure_logging
         configure_logging(console_log_level='info')
         if not path:
             path = os.path.join(os.path.expanduser('~/.esmvaltool'), filename)
@@ -191,7 +191,8 @@ def list():
         """
         import os
 
-        from ._config import DIAGNOSTICS_PATH, configure_logging
+        from ._config import DIAGNOSTICS_PATH
+        from ._logging import configure_logging
         configure_logging(console_log_level='info')
         recipes_folder = os.path.join(DIAGNOSTICS_PATH, 'recipes')
         logger.info("Showing recipes installed in %s", recipes_folder)
@@ -220,7 +221,8 @@ def get(recipe):
         import os
         import shutil
 
-        from ._config import DIAGNOSTICS_PATH, configure_logging
+        from ._config import DIAGNOSTICS_PATH
+        from ._logging import configure_logging
         configure_logging(console_log_level='info')
         installed_recipe = os.path.join(DIAGNOSTICS_PATH, 'recipes', recipe)
         if not os.path.exists(installed_recipe):
@@ -244,7 +246,8 @@ def show(recipe):
         """
         import os
 
-        from ._config import DIAGNOSTICS_PATH, configure_logging
+        from ._config import DIAGNOSTICS_PATH
+        from ._logging import configure_logging
         configure_logging(console_log_level='info')
         installed_recipe = os.path.join(DIAGNOSTICS_PATH, 'recipes', recipe)
         if not os.path.exists(installed_recipe):
@@ -344,11 +347,8 @@ def run(recipe,
         import os
         import shutil
 
-        from ._config import (
-            DIAGNOSTICS_PATH,
-            configure_logging,
-            read_config_user_file,
-        )
+        from ._config import DIAGNOSTICS_PATH, read_config_user_file
+        from ._logging import configure_logging
         from ._recipe import TASKSEP
         from .cmor.check import CheckLevels
 

tests/unit/test_logging.py

@@ -0,0 +1,38 @@
+"""Tests for logging config submodule."""
+
+import logging
+from pathlib import Path
+
+import pytest
+
+from esmvalcore._logging import configure_logging
+
+
+@pytest.mark.parametrize('level', (None, 'INFO', 'DEBUG'))
+def test_logging_with_level(level):
+    """Test log level configuration."""
+    ret = configure_logging(console_log_level=level)
+    assert isinstance(ret, list)
+    assert len(ret) == 0
+
+    root = logging.getLogger()
+
+    assert len(root.handlers) == 1
+
+
+def test_logging_with_output_dir(tmp_path):
+    """Test that paths are configured."""
+    ret = configure_logging(output_dir=tmp_path)
+    assert isinstance(ret, list)
+    for path in ret:
+        assert tmp_path == Path(path).parent
+
+    root = logging.getLogger()
+
+    assert len(root.handlers) == len(ret) + 1
+
+
+def test_logging_log_level_invalid():
+    """Test failure condition for invalid level specification."""
+    with pytest.raises(ValueError):
+        configure_logging(console_log_level='FAIL')