Merge branch 'SAR-55-fix-default-formatter-time' into 'master'

SAR-55:  Fix default formatter time

See merge request ska-telescope/ska-logging!3

ska_logging/__init__.py

@@ -5,8 +5,6 @@
 __all__ = (
     "configure_logging",
     "get_default_formatter",
-    "SkaLoggingError",
-    "SkaLoggingTagsFormatError",
 )
 __author__ = "Anton Joubert"
 __email__ = "ajoubert+ska@ska.ac.za"
@@ -15,8 +13,6 @@ __email__ = "ajoubert+ska@ska.ac.za"
 from .configuration import (
     configure_logging,
     get_default_formatter,
-    SkaLoggingError,
-    SkaLoggingTagsFormatError,
 )
 
 

ska_logging/configuration.py

@@ -7,14 +7,6 @@ import logging.config
 import time
 
 
-class SkaLoggingError(Exception):
-    """Base class for all SKA Logger exceptions."""
-
-
-class SkaLoggingTagsFormatError(SkaLoggingError):
-    """Invalid format for the 'tags' field string."""
-
-
 class _UTCFormatter(logging.Formatter):
     converter = time.gmtime
 
@@ -46,7 +38,7 @@ _FORMAT_STR_WITH_TAGS = (
     "%(message)s"
 )
 
-_INVALID_TAG_CHARS = ("|", "%")
+_FORMAT_STR_DATE = "%Y-%m-%dT%H:%M:%S"
 
 _LOGGING_CONFIG = {
     "version": 1,
@@ -55,7 +47,7 @@ _LOGGING_CONFIG = {
         "default": {
             "()": _UTCFormatter,
             "format": _FORMAT_STR_NO_TAGS,
-            "datefmt": "%Y-%m-%dT%H:%M:%S",
+            "datefmt": _FORMAT_STR_DATE,
         }
     },
     "handlers": {
@@ -103,7 +95,7 @@ def configure_logging(level=None, tags_filter=None, overrides=None):
     """
     config = copy.deepcopy(_LOGGING_CONFIG)
     if overrides:
-        config = _override(config, overrides)
+        config = _override(config, copy.deepcopy(overrides))
     if level:
         config["root"]["level"] = level
     if tags_filter:
@@ -123,37 +115,21 @@ def get_default_formatter(tags=False):
     Parameters
     ----------
 
-    tags : bool or str, optional
-        If boolean, then treated as a toggle:
-          - True:  include the "tags" field in the format string.  This requires
+    tags : bool, optional
+        If true, then include the "tags" field in the format string.  This requires
         a tags filter to be linked to the corresponding handler.
-          - False:  exclude the "tags" field from the format string.
-        If string, then it is a static tag.  Instead of using a logging filter, the
-        formatter will just use this static string for the "tags" field directly.
 
     Returns
     -------
     logging.Formatter
         A new default formatter.
 
-    Raises
-    ------
-    SkaLoggingTagsFormatError:
-        If the static tags string has an invalid format.
-
     """
-    if isinstance(tags, str):
-        invalid_chars = [c for c in _INVALID_TAG_CHARS if c in tags]
-        if invalid_chars:
-            raise SkaLoggingTagsFormatError(
-                "Invalid char(s) {} in tags: {!r}".format(invalid_chars, tags)
-            )
-        format_str = _FORMAT_STR_WITH_TAGS.replace("%(tags)s", tags)
-    elif tags:
+    if tags:
         format_str = _FORMAT_STR_WITH_TAGS
     else:
         format_str = _FORMAT_STR_NO_TAGS
-    return _UTCFormatter(fmt=format_str)
+    return _UTCFormatter(fmt=format_str, datefmt=_FORMAT_STR_DATE)
 
 
 def _override(config, overrides):

tests/test_configuration.py

@@ -9,7 +9,7 @@ import pytest
 
 import ska_logging.configuration
 
-from ska_logging import configure_logging, get_default_formatter, SkaLoggingTagsFormatError
+from ska_logging import configure_logging, get_default_formatter
 
 
 @pytest.fixture
@@ -67,18 +67,15 @@ RECORDER_OVERRIDES = {
 }
 
 
-@pytest.mark.usefixtures("reset_logging")
-class TestConfigureLogging:
-    """Tests for :func:`~ska_logging.configuration.configure_logging`."""
-
 @pytest.fixture
-    def default_logger(self):
+def default_logger():
     """Return user logger instance with default configuration."""
     configure_logging()
     yield logging.getLogger("ska.test.app")
 
+
 @pytest.fixture
-    def recording_logger(self):
+def recording_logger():
     """Return user logger, including a recording handler.
 
     The additional handler has the name "recorder".  It uses the default formatter,
@@ -91,8 +88,9 @@ class TestConfigureLogging:
     configure_logging(overrides=RECORDER_OVERRIDES)
     yield logging.getLogger("ska.logger")
 
+
 @pytest.fixture
-    def recording_tags_logger(self):
+def recording_tags_logger():
     """Return user logger like :func:`recording_logger`, but including tags filter."""
 
     class MyFilter(logging.Filter):
@@ -103,6 +101,11 @@ class TestConfigureLogging:
     configure_logging(tags_filter=MyFilter, overrides=RECORDER_OVERRIDES)
     yield logging.getLogger("ska.logger")
 
+
+@pytest.mark.usefixtures("reset_logging")
+class TestConfigureLogging:
+    """Tests for :func:`~ska_logging.configuration.configure_logging`."""
+
     def test_includes_console_handler(self, default_logger):
         assert get_named_handler(default_logger, "console")
 
@@ -165,36 +168,34 @@ class TestConfigureLogging:
         assert get_named_handler(logger, "test")
 
 
+@pytest.mark.usefixtures("reset_logging")
 class TestGetDefaultFormatter:
     """Tests for :func:`~ska_logging.configuration.get_default_formatter`."""
 
-    def test_default_no_tags(self):
+    def get_recorded_message(self, logger):
+        logger.info("test message")
+        recorder = get_named_handler(logger, "recorder")
+        record = recorder.records[0]
+        message = recorder.logs[0]
+        return record, message
+
+    def test_default_no_tags(self, recording_logger):
         formatter = get_default_formatter()
-        assert isinstance(formatter, ska_logging.configuration._UTCFormatter)
-        assert formatter._fmt == ska_logging.configuration._FORMAT_STR_NO_TAGS
+        record, message = self.get_recorded_message(recording_logger)
+        actual_log_message = formatter.format(record)
+        assert actual_log_message == message
 
-    def test_get_tags_disabled(self):
+    def test_get_tags_disabled(self, recording_logger):
         formatter = get_default_formatter(tags=False)
-        assert isinstance(formatter, ska_logging.configuration._UTCFormatter)
-        assert formatter._fmt == ska_logging.configuration._FORMAT_STR_NO_TAGS
+        record, message = self.get_recorded_message(recording_logger)
+        actual_log_message = formatter.format(record)
+        assert actual_log_message == message
 
-    def test_get_tags_enabled(self):
+    def test_get_tags_enabled(self, recording_tags_logger):
         formatter = get_default_formatter(tags=True)
-        assert isinstance(formatter, ska_logging.configuration._UTCFormatter)
-        assert formatter._fmt == ska_logging.configuration._FORMAT_STR_WITH_TAGS
-
-    def test_get_tags_static_string(self):
-        formatter = get_default_formatter(tags="test-key:test-value")
-        assert isinstance(formatter, ska_logging.configuration._UTCFormatter)
-        tags_format = ska_logging.configuration._FORMAT_STR_WITH_TAGS
-        expected_format = tags_format.replace("%(tags)s", "test-key:test-value")
-        assert formatter._fmt == expected_format
-
-    def test_get_tags_invalid_static_string(self):
-        with pytest.raises(SkaLoggingTagsFormatError):
-            get_default_formatter(tags="no|pipes|allowed")
-        with pytest.raises(SkaLoggingTagsFormatError):
-            get_default_formatter(tags="no%percentage%symbols%allowed")
+        record, message = self.get_recorded_message(recording_tags_logger)
+        actual_log_message = formatter.format(record)
+        assert actual_log_message == message
 
 
 class TestOverride: