From 12680b1d7498f1b521912871cbad5368f236a7e1 Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 12 Sep 2024 19:48:37 -0700 Subject: [PATCH 1/3] Improved logic for concatenating message, prefix, and suffix in bittensor logging + test --- bittensor/utils/btlogging/loggingmachine.py | 30 ++++++++++++--------- tests/unit_tests/test_logging.py | 22 ++++++++++++++- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/bittensor/utils/btlogging/loggingmachine.py b/bittensor/utils/btlogging/loggingmachine.py index 3fd323c74..b8569648d 100644 --- a/bittensor/utils/btlogging/loggingmachine.py +++ b/bittensor/utils/btlogging/loggingmachine.py @@ -30,7 +30,7 @@ import sys from logging import Logger from logging.handlers import QueueHandler, QueueListener, RotatingFileHandler -from typing import NamedTuple +from typing import NamedTuple, Optional from statemachine import State, StateMachine @@ -47,6 +47,12 @@ from .helpers import all_loggers +def _concat_message(msg: str, prefix: Optional[str] = None, suffix: Optional[str] = None): + """Concatenates a message with optional prefix and suffix.""" + msg = f"{f'{prefix} - ' if prefix else ''}{msg}{f' - {suffix}' if suffix else ''}" + return msg + + class LoggingConfig(NamedTuple): """Named tuple to hold the logging configuration.""" @@ -361,42 +367,42 @@ def __trace_on__(self) -> bool: """ return self.current_state_value == "Trace" - def trace(self, msg="", prefix="", suffix="", *args, **kwargs): + def trace(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): """Wraps trace message with prefix and suffix.""" - msg = f"{prefix} - {msg} - {suffix}" + msg = _concat_message(msg, prefix, suffix) self._logger.trace(msg, *args, **kwargs) - def debug(self, msg="", prefix="", suffix="", *args, **kwargs): + def debug(self, msg="", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): """Wraps debug message with prefix and suffix.""" - msg = f"{prefix} - {msg} - {suffix}" + msg = _concat_message(msg, prefix, suffix) self._logger.debug(msg, *args, **kwargs) - def info(self, msg="", prefix="", suffix="", *args, **kwargs): + def info(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): """Wraps info message with prefix and suffix.""" - msg = f"{prefix} - {msg} - {suffix}" + msg = _concat_message(msg, prefix, suffix) self._logger.info(msg, *args, **kwargs) - def success(self, msg="", prefix="", suffix="", *args, **kwargs): + def success(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): """Wraps success message with prefix and suffix.""" msg = f"{prefix} - {msg} - {suffix}" self._logger.success(msg, *args, **kwargs) - def warning(self, msg="", prefix="", suffix="", *args, **kwargs): + def warning(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): """Wraps warning message with prefix and suffix.""" msg = f"{prefix} - {msg} - {suffix}" self._logger.warning(msg, *args, **kwargs) - def error(self, msg="", prefix="", suffix="", *args, **kwargs): + def error(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): """Wraps error message with prefix and suffix.""" msg = f"{prefix} - {msg} - {suffix}" self._logger.error(msg, *args, **kwargs) - def critical(self, msg="", prefix="", suffix="", *args, **kwargs): + def critical(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): """Wraps critical message with prefix and suffix.""" msg = f"{prefix} - {msg} - {suffix}" self._logger.critical(msg, *args, **kwargs) - def exception(self, msg="", prefix="", suffix="", *args, **kwargs): + def exception(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): """Wraps exception message with prefix and suffix.""" msg = f"{prefix} - {msg} - {suffix}" self._logger.exception(msg, *args, **kwargs) diff --git a/tests/unit_tests/test_logging.py b/tests/unit_tests/test_logging.py index 6bd7b6acd..15b41001b 100644 --- a/tests/unit_tests/test_logging.py +++ b/tests/unit_tests/test_logging.py @@ -9,7 +9,7 @@ DEFAULT_LOG_FILE_NAME, BITTENSOR_LOGGER_NAME, ) -from bittensor.utils.btlogging.loggingmachine import LoggingConfig +from bittensor.utils.btlogging.loggingmachine import LoggingConfig, _concat_message @pytest.fixture(autouse=True, scope="session") @@ -175,3 +175,23 @@ def test_all_log_levels_output(logging_machine, caplog): assert "Test warning" in caplog.text assert "Test error" in caplog.text assert "Test critical" in caplog.text + + +@pytest.mark.parametrize( + "msg, prefix, suffix, expected_result", + [ + ("msg", None, None, "msg"), + ("msg", "prefix", None, "prefix - msg"), + ("msg", None, "suffix", "msg - suffix"), + ("msg", "prefix", "suffix", "prefix - msg - suffix"), + ], + ids=[ + "message, no prefix, no suffix", + "message and prefix only", + "message and suffix only", + "message, prefix, and suffix", + ], +) +def test_concat(msg, prefix, suffix, expected_result): + """Test different options of message concatenation with prefix and suffix.""" + assert _concat_message(msg, prefix, suffix) == expected_result From 38f4eb85ac40d719dc619a61481c68d3493dd93c Mon Sep 17 00:00:00 2001 From: Roman Date: Thu, 12 Sep 2024 20:08:25 -0700 Subject: [PATCH 2/3] fix + ruff --- bittensor/utils/btlogging/loggingmachine.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/bittensor/utils/btlogging/loggingmachine.py b/bittensor/utils/btlogging/loggingmachine.py index b8569648d..8e12c7056 100644 --- a/bittensor/utils/btlogging/loggingmachine.py +++ b/bittensor/utils/btlogging/loggingmachine.py @@ -30,7 +30,7 @@ import sys from logging import Logger from logging.handlers import QueueHandler, QueueListener, RotatingFileHandler -from typing import NamedTuple, Optional +from typing import NamedTuple from statemachine import State, StateMachine @@ -47,7 +47,7 @@ from .helpers import all_loggers -def _concat_message(msg: str, prefix: Optional[str] = None, suffix: Optional[str] = None): +def _concat_message(msg="", prefix="", suffix=""): """Concatenates a message with optional prefix and suffix.""" msg = f"{f'{prefix} - ' if prefix else ''}{msg}{f' - {suffix}' if suffix else ''}" return msg @@ -367,42 +367,42 @@ def __trace_on__(self) -> bool: """ return self.current_state_value == "Trace" - def trace(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): + def trace(self, msg="", prefix="", suffix="", *args, **kwargs): """Wraps trace message with prefix and suffix.""" msg = _concat_message(msg, prefix, suffix) self._logger.trace(msg, *args, **kwargs) - def debug(self, msg="", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): + def debug(self, msg="", prefix="", suffix="", *args, **kwargs): """Wraps debug message with prefix and suffix.""" msg = _concat_message(msg, prefix, suffix) self._logger.debug(msg, *args, **kwargs) - def info(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): + def info(self, msg="", prefix="", suffix="", *args, **kwargs): """Wraps info message with prefix and suffix.""" msg = _concat_message(msg, prefix, suffix) self._logger.info(msg, *args, **kwargs) - def success(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): + def success(self, msg="", prefix="", suffix="", *args, **kwargs): """Wraps success message with prefix and suffix.""" msg = f"{prefix} - {msg} - {suffix}" self._logger.success(msg, *args, **kwargs) - def warning(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): + def warning(self, msg="", prefix="", suffix="", *args, **kwargs): """Wraps warning message with prefix and suffix.""" msg = f"{prefix} - {msg} - {suffix}" self._logger.warning(msg, *args, **kwargs) - def error(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): + def error(self, msg="", prefix="", suffix="", *args, **kwargs): """Wraps error message with prefix and suffix.""" msg = f"{prefix} - {msg} - {suffix}" self._logger.error(msg, *args, **kwargs) - def critical(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): + def critical(self, msg="", prefix="", suffix="", *args, **kwargs): """Wraps critical message with prefix and suffix.""" msg = f"{prefix} - {msg} - {suffix}" self._logger.critical(msg, *args, **kwargs) - def exception(self, msg: str = "", prefix: Optional[str] = None, suffix: Optional[str] = None, *args, **kwargs): + def exception(self, msg="", prefix="", suffix="", *args, **kwargs): """Wraps exception message with prefix and suffix.""" msg = f"{prefix} - {msg} - {suffix}" self._logger.exception(msg, *args, **kwargs) From 6f2ce29437dcea1f5e5b15183dd97de8f6512bf6 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 13 Sep 2024 09:15:11 -0700 Subject: [PATCH 3/3] improve btlogging concat test --- tests/unit_tests/test_logging.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/test_logging.py b/tests/unit_tests/test_logging.py index 15b41001b..2c5e593f0 100644 --- a/tests/unit_tests/test_logging.py +++ b/tests/unit_tests/test_logging.py @@ -180,13 +180,15 @@ def test_all_log_levels_output(logging_machine, caplog): @pytest.mark.parametrize( "msg, prefix, suffix, expected_result", [ + ("msg", "", "", "msg"), ("msg", None, None, "msg"), ("msg", "prefix", None, "prefix - msg"), ("msg", None, "suffix", "msg - suffix"), ("msg", "prefix", "suffix", "prefix - msg - suffix"), ], ids=[ - "message, no prefix, no suffix", + "message, no prefix (str), no suffix (str)", + "message, no prefix (None), no suffix (None)", "message and prefix only", "message and suffix only", "message, prefix, and suffix",