diff --git a/doc/changes/DM-42273.api.md b/doc/changes/DM-42273.api.md new file mode 100644 index 0000000000..11126171ca --- /dev/null +++ b/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() diff --git a/doc/changes/DM-42273.removal.md b/doc/changes/DM-42273.removal.md new file mode 100644 index 0000000000..c14580f9ab --- /dev/null +++ b/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. diff --git a/python/lsst/daf/butler/cli/cliLog.py b/python/lsst/daf/butler/cli/cliLog.py index cfab0cbe7c..7840d137de 100644 --- a/python/lsst/daf/butler/cli/cliLog.py +++ b/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,8 +328,7 @@ 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()) @@ -305,8 +336,8 @@ def setLogLevels(cls, logLevels: list[tuple[str | None, str]] | dict[str, str]) # 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) diff --git a/requirements.txt b/requirements.txt index 908c8cb743..e709017868 100644 --- a/requirements.txt +++ b/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 diff --git a/tests/test_cliLogLogState.py b/tests/test_cliLogLogState.py new file mode 100644 index 0000000000..d1ef2182ac --- /dev/null +++ b/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 . + +"""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()