From 4de3314d97964a708122cfd5ac277026cd4be891 Mon Sep 17 00:00:00 2001
From: Brett <brettgraham@gmail.com>
Date: Tue, 23 Jul 2024 13:08:43 -0400
Subject: [PATCH 1/2] conditionally format log_records

Instead of storing every log message as a record,
only store the formatted logs (if a formatter was provided).
A formatter can be supplied by setting `_log_records_formatter`
on the `Step` class/instance.

Co-authored-by: Zach Burnett <zachary.r.burnett@gmail.com>
---
 src/stpipe/log.py    | 23 ++++++++++++++---------
 src/stpipe/step.py   |  8 ++++++--
 tests/test_logger.py |  8 +++++---
 tests/test_step.py   |  5 ++---
 4 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/stpipe/log.py b/src/stpipe/log.py
index b3fc93b7..8c55bf77 100644
--- a/src/stpipe/log.py
+++ b/src/stpipe/log.py
@@ -280,18 +280,23 @@ def log_records(self):
         return self._log_records
 
     def emit(self, record):
-        self._log_records.append(record)
+        if self.formatter is not None:
+            self._log_records.append(self.formatter.format(record))
 
 
 @contextmanager
-def record_logs(level=logging.NOTSET):
-    handler = RecordingHandler(level=level)
-    logger = getLogger(STPIPE_ROOT_LOGGER)
-    logger.addHandler(handler)
-    try:
-        yield handler.log_records
-    finally:
-        logger.removeHandler(handler)
+def record_logs(level=logging.NOTSET, formatter=None):
+    if formatter is None:
+        yield []
+    else:
+        handler = RecordingHandler(level=level)
+        handler.setFormatter(formatter)
+        logger = getLogger(STPIPE_ROOT_LOGGER)
+        logger.addHandler(handler)
+        try:
+            yield handler.log_records
+        finally:
+            logger.removeHandler(handler)
 
 
 # Install the delegation handler on the root logger.  The Step class
diff --git a/src/stpipe/step.py b/src/stpipe/step.py
index b2c6b42b..adf5cc3d 100644
--- a/src/stpipe/step.py
+++ b/src/stpipe/step.py
@@ -75,6 +75,10 @@ class Step:
     # but by default attempt to prefetch
     prefetch_references = True
 
+    # This needs to be set to a logging formatter for any
+    # log_records to be saved.
+    _log_records_formatter = None
+
     @classmethod
     def get_config_reftype(cls):
         """
@@ -405,7 +409,7 @@ def log_records(self):
 
         Returns
         -------
-        list of logging.LogRecord
+        list of str
         """
         return self._log_records
 
@@ -417,7 +421,7 @@ def run(self, *args):
         """
         gc.collect()
 
-        with log.record_logs() as log_records:
+        with log.record_logs(formatter=self._log_records_formatter) as log_records:
             self._log_records = log_records
 
             # Make generic log messages go to this step's logger
diff --git a/tests/test_logger.py b/tests/test_logger.py
index ccaf013e..a8e7bed5 100644
--- a/tests/test_logger.py
+++ b/tests/test_logger.py
@@ -53,7 +53,9 @@ def test_record_logs():
         isinstance(h, stpipe_log.RecordingHandler) for h in root_logger.handlers
     )
 
-    with stpipe_log.record_logs(level=logging.ERROR) as log_records:
+    with stpipe_log.record_logs(
+        level=logging.ERROR, formatter=logging.Formatter("%(message)s")
+    ) as log_records:
         stpipe_logger.warning("Warning from stpipe")
         stpipe_logger.error("Error from stpipe")
         root_logger.warning("Warning from root")
@@ -67,5 +69,5 @@ def test_record_logs():
     root_logger.error("Additional error from root")
 
     assert len(log_records) == 2
-    assert log_records[0].message == "Error from stpipe"
-    assert log_records[1].message == "Error from root"
+    assert log_records[0] == "Error from stpipe"
+    assert log_records[1] == "Error from root"
diff --git a/tests/test_step.py b/tests/test_step.py
index f271a999..972f84a9 100644
--- a/tests/test_step.py
+++ b/tests/test_step.py
@@ -51,6 +51,7 @@ class LoggingPipeline(Pipeline):
         str1 = string(default='default')
         output_ext = string(default='simplestep')
     """
+    _log_records_formatter = logging.Formatter("%(message)s")
 
     def process(self):
         self.log.warning("This step has called out a warning.")
@@ -409,6 +410,4 @@ def test_log_records():
     pipeline = LoggingPipeline()
     pipeline.run()
 
-    assert any(
-        r.message == "This step has called out a warning." for r in pipeline.log_records
-    )
+    assert any(r == "This step has called out a warning." for r in pipeline.log_records)

From a81a41248e099acadce667dcb25e0cdb963a5e13 Mon Sep 17 00:00:00 2001
From: Brett <brettgraham@gmail.com>
Date: Thu, 25 Jul 2024 09:48:12 -0400
Subject: [PATCH 2/2] add changelog

---
 CHANGES.rst | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index 8cf27fcf..fecc301e 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,9 +1,13 @@
-0.6.1 (unreleased)
+0.7.0 (unreleased)
 ==================
 
 - remove Windows tests and add info box indicating lack of Windows support to README [#163]
 - add ``ModelLibrary`` container class [#156]
 
+0.6.1 (unreleased)
+==================
+- log only strings in ``Step.log_records`` when a formatter is provided [#171]
+
 0.6.0 (2024-01-24)
 ==================