tickets/DM-42273 Refactor CliLog to move library code to new class LogState

doc/changes/DM-42273.api.md

@@ -0,0 +1 @@
+Change to CliLog API to add lsst.utils.logging.LogState, which allows access to configuration state without accessing the configState member directly through CliLog.  New methods are LogState.record(), LogState.get_state(), LogState.get_state(), LogState.clear_state(), LogState.replay_state()

doc/changes/DM-42273.removal.md

@@ -0,0 +1 @@
+Deprecations of setting or using CliLog.configState directly.  This should be done via LogState.set_state() and LogState.get_state() directly.

python/lsst/daf/butler/cli/cliLog.py

@@ -34,8 +34,11 @@
 import datetime
 import logging
 import os
+import warnings
 from typing import Any
 
+from lsst.utils.logging import LogState
+
 try:
     import lsst.log as lsstLog
 except ModuleNotFoundError:
@@ -61,8 +64,8 @@ def formatTime(self, record: logging.LogRecord, datefmt: str | None = None) -> s
         ----------
         record : `logging.LogRecord`
             The record to format.
-        datefmt : `str` or `None`, optional
-            Format to use when formatting the date.
+        datefmt : `str`, optional
+            An explicit date format, or `None` for ISO 8601.
 
         Returns
         -------
@@ -79,7 +82,33 @@ def formatTime(self, record: logging.LogRecord, datefmt: str | None = None) -> s
         return s
 
 
-class CliLog:
+class _ConfigStateDescriptor(type):
+    """Descriptor to intercept direct access to CliLog.configState."""
+
+    def __getattribute__(cls, name: str) -> Any:
+        if name == "configState":
+            warnings.warn(
+                "CliLog.configState is deprecated; use LogState.get_state() instead.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+            return LogState.configState
+        return super().__getattribute__(name)
+
+    def __setattr__(cls, name: str, value: Any) -> None:
+        if name == "configState":
+            warnings.warn(
+                "Assigning to CliLog.configState is deprecated; "
+                "use LogState.clear_state() or LogState.record_* instead.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+            LogState.set_state(value)
+            return
+        super().__setattr__(name, value)
+
+
+class CliLog(metaclass=_ConfigStateDescriptor):
     """Interface for managing python logging and ``lsst.log``.
 
     This class defines log format strings for the log output and timestamp
@@ -101,13 +130,16 @@ class CliLog:
     """The log format used when the lsst.log package is not importable and the
     log is initialized with longlog=False."""
 
-    configState: list[tuple[Any, ...]] = []
+    configState: Any = None
     """Configuration state. Contains tuples where first item in a tuple is
     a method and remaining items are arguments for the method.
+
+    This attribute is retained for backwards compatibility only. New code
+    should use :class:`LogState` instead.
     """
 
     _initialized = False
-    _componentSettings: list[ComponentSettings] = []
+    _componentSettings: list[ComponentSettings] = []  # type: ignore[name-defined]
 
     _fileHandlers: list[logging.FileHandler] = []
     """Any FileHandler classes attached to the root logger by this class
@@ -244,8 +276,8 @@ def initLog(
             for key, value in log_label.items():
                 ButlerMDC.MDC(key.upper(), value)
 
-        # remember this call
-        cls.configState.append((cls.initLog, longlog, log_tty, log_file, log_label))
+        # remember this call in the library recorder, not on CliLog itself
+        LogState.record((CliLog.initLog, longlog, log_tty, log_file, log_label))
 
     @classmethod
     def resetLog(cls) -> None:
@@ -280,7 +312,7 @@ def resetLog(cls) -> None:
 
         cls._fileHandlers.clear()
         cls._initialized = False
-        cls.configState = []
+        LogState.clear_state()
 
     @classmethod
     def setLogLevels(cls, logLevels: list[tuple[str | None, str]] | dict[str, str]) -> None:
@@ -296,17 +328,16 @@ def setLogLevels(cls, logLevels: list[tuple[str | None, str]] | dict[str, str])
 
         Notes
         -----
-        The special name ``.`` can be used to set the Python root
-        logger.
+        The special name ``.`` can be used to set the Python root logger.
         """
         if isinstance(logLevels, dict):
             logLevels = list(logLevels.items())
 
         # configure individual loggers
         for component, level in logLevels:
             cls._setLogLevel(component, level)
-            # remember this call
-            cls.configState.append((cls._setLogLevel, component, level))
+            # remember this call in the *library* recorder
+            LogState.record((CliLog._setLogLevel, component, level))
 
     @classmethod
     def _setLogLevel(cls, component: str | None, level: str) -> None:
@@ -316,13 +347,13 @@ def _setLogLevel(cls, component: str | None, level: str) -> None:
 
         Parameters
         ----------
-        component : `str` or None
-            The name of the log component or `None` for the default logger.
-            The root logger can be specified either by an empty string or
-            with the special name ``.``.
+        component : `str` or `None`
+            Component name or `None` for default root logger.
         level : `str`
-            A valid python logging level.
+            Log level name.
         """
+        level = level.upper()
+
         components: set[str | None]
         if component is None:
             components = set(cls.root_loggers())
@@ -427,21 +458,20 @@ def _recordComponentSetting(cls, component: str | None) -> None:
 
     @classmethod
     def replayConfigState(cls, configState: list[tuple[Any, ...]]) -> None:
-        """Re-create configuration using configuration state recorded earlier.
+        """Recreate configuration using recorded config state.
+
+        This method is preserved for backwards compatibility only and will be
+        removed in a future release. New code should use
+        `LogState.replay_state`.
 
         Parameters
         ----------
         configState : `list` of `tuple`
-            Tuples contain a method as first item and arguments for the method,
-            in the same format as ``cls.configState``.
+            Tuples contain a method as first item and arguments for the method.
         """
-        if cls._initialized or cls.configState:
-            # Already initialized, do not touch anything.
-            log = logging.getLogger(__name__)
-            log.warning("Log is already initialized, will not replay configuration.")
-            return
-
-        # execute each one in order
-        for call in configState:
-            method, *args = call
-            method(*args)
+        warnings.warn(
+            "CliLog.replayConfigState is deprecated; use LogState.replay_state instead.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        LogState.replay_state(configState)

requirements.txt

@@ -2,5 +2,5 @@
 -r requirements/test.in
 
 lsst-sphgeom @ git+https://github.com/lsst/sphgeom@main
-lsst-utils @ git+https://github.com/lsst/utils@main
+lsst-utils @ git+https://github.com/lsst/utils@tickets/DM-42273
 lsst-resources[https,s3] @ git+https://github.com/lsst/resources@main

tests/test_cliLogLogState.py

@@ -0,0 +1,111 @@
+# This file is part of daf_butler.
+#
+# Developed for the LSST Data Management System.
+# This product includes software developed by the LSST Project
+# (http://www.lsst.org).
+# See the COPYRIGHT file at the top-level directory of this distribution
+# for details of code ownership.
+#
+# This software is dual licensed under the GNU General Public License and also
+# under a 3-clause BSD license. Recipients may choose which of these licenses
+# to use; please see the files gpl-3.0.txt and/or bsd_license.txt,
+# respectively.  If you choose the GPL option then the following text applies
+# (but note that there is still no warranty even if you opt for BSD instead):
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Unit tests for the daf_butler dataset-type CLI option."""
+
+import unittest
+
+from lsst.daf.butler.cli.cliLog import CliLog
+from lsst.utils.logging import LogState
+
+try:
+    import lsst.log as lsstLog
+except ModuleNotFoundError:
+    lsstLog = None
+
+
+class LogStateTestCase(unittest.TestCase):
+    """Test python command-line log levels."""
+
+    def test_state(self):
+        """Tests states"""
+        CliLog.resetLog()
+        CliLog.initLog(False, True, (), ())
+
+        state_list = LogState.get_state()
+        first = state_list[0]
+
+        self.assertEqual(first[0].__func__, CliLog.initLog.__func__)
+        self.assertEqual(first[1], False)
+        self.assertEqual(first[2], True)
+        self.assertEqual(first[3], ())
+        self.assertEqual(first[4], ())
+
+        LogState.clear_state()
+        state_list = LogState.get_state()
+        self.assertEqual(len(state_list), 0)
+
+    def test_levels(self):
+        """Tests setting LogLevel in CliLog and retrieving it from LogState"""
+        CliLog.resetLog()
+        CliLog.setLogLevels([(None, "CRITICAL")])
+        state_list = LogState.get_state()
+        first = state_list[0]
+
+        self.assertEqual(first[0].__func__, CliLog._setLogLevel.__func__)
+        self.assertEqual(first[1], None)
+        self.assertEqual(first[2], "CRITICAL")
+
+    def test_replay(self):
+        """Tests replay_log_state"""
+        CliLog.resetLog()
+
+        CliLog.setLogLevels([(None, "WARNING")])
+
+        state_list = LogState.get_state()
+        LogState.clear_state()
+
+        LogState.replay_state(state_list)
+        with self.assertLogs(None, level="WARN"):
+            LogState.replay_state(state_list)
+        LogState.clear_state()
+
+        LogState.replay_state(state_list)
+        state_list = LogState.get_state()
+        self.assertEqual(len(state_list), 0)
+
+        CliLog.setLogLevels([(None, "WARNING")])
+        state_list = LogState.get_state()
+        with self.assertWarns(DeprecationWarning):
+            CliLog.replayConfigState(state_list)
+
+    def test_deprecations(self):
+        """Test deprecated accessor in CliLog"""
+        CliLog.resetLog()
+
+        CliLog.setLogLevels([(None, "WARNING")])
+        state_list = LogState.get_state()
+
+        with self.assertWarns(DeprecationWarning):
+            x = CliLog.configState  # noqa: F841
+
+        with self.assertWarns(DeprecationWarning):
+            CliLog.configState = state_list
+
+
+if __name__ == "__main__":
+    unittest.main()