diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 767509c9a9..bb888d520a 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,2 +1,5 @@ # assign the entire repo to the maintainers team * @beetbox/maintainers + +# Specific ownerships: +/beets/metadata_plugins.py @semohr \ No newline at end of file diff --git a/beets/logging.py b/beets/logging.py index fd8b1962ff..3ed5e5a843 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -20,6 +20,8 @@ calls (`debug`, `info`, etc). """ +from __future__ import annotations + import threading from copy import copy from logging import ( @@ -32,8 +34,10 @@ Handler, Logger, NullHandler, + RootLogger, StreamHandler, ) +from typing import TYPE_CHECKING, Any, Mapping, TypeVar, Union, overload __all__ = [ "DEBUG", @@ -49,8 +53,20 @@ "getLogger", ] +if TYPE_CHECKING: + T = TypeVar("T") + from types import TracebackType + + # see https://github.com/python/typeshed/blob/main/stdlib/logging/__init__.pyi + _SysExcInfoType = Union[ + tuple[type[BaseException], BaseException, Union[TracebackType, None]], + tuple[None, None, None], + ] + _ExcInfoType = Union[None, bool, _SysExcInfoType, BaseException] + _ArgsType = Union[tuple[object, ...], Mapping[str, object]] + -def logsafe(val): +def _logsafe(val: T) -> str | T: """Coerce `bytes` to `str` to avoid crashes solely due to logging. This is particularly relevant for bytestring paths. Much of our code @@ -83,40 +99,45 @@ class StrFormatLogger(Logger): """ class _LogMessage: - def __init__(self, msg, args, kwargs): + def __init__( + self, + msg: str, + args: _ArgsType, + kwargs: dict[str, Any], + ): self.msg = msg self.args = args self.kwargs = kwargs def __str__(self): - args = [logsafe(a) for a in self.args] - kwargs = {k: logsafe(v) for (k, v) in self.kwargs.items()} + args = [_logsafe(a) for a in self.args] + kwargs = {k: _logsafe(v) for (k, v) in self.kwargs.items()} return self.msg.format(*args, **kwargs) def _log( self, - level, - msg, - args, - exc_info=None, - extra=None, - stack_info=False, + level: int, + msg: object, + args: _ArgsType, + exc_info: _ExcInfoType = None, + extra: Mapping[str, Any] | None = None, + stack_info: bool = False, + stacklevel: int = 1, **kwargs, ): """Log msg.format(*args, **kwargs)""" - m = self._LogMessage(msg, args, kwargs) - stacklevel = kwargs.pop("stacklevel", 1) - stacklevel = {"stacklevel": stacklevel} + if isinstance(msg, str): + msg = self._LogMessage(msg, args, kwargs) return super()._log( level, - m, + msg, (), exc_info=exc_info, extra=extra, stack_info=stack_info, - **stacklevel, + stacklevel=stacklevel, ) @@ -156,9 +177,12 @@ class BeetsLogger(ThreadLocalLevelLogger, StrFormatLogger): my_manager.loggerClass = BeetsLogger -# Override the `getLogger` to use our machinery. -def getLogger(name=None): # noqa +@overload +def getLogger(name: str) -> BeetsLogger: ... +@overload +def getLogger(name: None = ...) -> RootLogger: ... +def getLogger(name=None) -> BeetsLogger | RootLogger: # noqa: N802 if name: - return my_manager.getLogger(name) + return my_manager.getLogger(name) # type: ignore[return-value] else: return Logger.root diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 54c065da40..37e7426f62 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -36,10 +36,10 @@ if TYPE_CHECKING: from collections.abc import Iterable, Iterator, Sequence - from logging import Logger from beets.importer import ImportSession, ImportTask from beets.library import Album, Library + from beets.logging import BeetsLogger as Logger try: from bs4 import BeautifulSoup, Tag diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index f492ab3ccd..d245d6a14c 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -42,10 +42,9 @@ from beets.util.config import sanitize_choices if TYPE_CHECKING: - from logging import Logger - from beets.importer import ImportTask from beets.library import Item, Library + from beets.logging import BeetsLogger as Logger from ._typing import ( GeniusAPI, @@ -186,7 +185,7 @@ def slug(text: str) -> str: class RequestHandler: - _log: beets.logging.Logger + _log: Logger def debug(self, message: str, *args) -> None: """Log a debug message with the class name.""" diff --git a/docs/changelog.rst b/docs/changelog.rst index 52e9354450..38037955ea 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -51,6 +51,12 @@ Other changes: - :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific disambiguation stripping. +For developers and plugin authors: + +- Typing improvements in ``beets/logging.py``: ``getLogger`` now returns + ``BeetsLogger`` when called with a name, or ``RootLogger`` when called without + a name. + 2.4.0 (September 13, 2025) -------------------------- diff --git a/test/test_logging.py b/test/test_logging.py index b751dd70ec..48f9cbfd89 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -3,19 +3,21 @@ import logging as log import sys import threading -import unittest -from io import StringIO from types import ModuleType from unittest.mock import patch +import pytest + import beets.logging as blog from beets import plugins, ui from beets.test import _common, helper from beets.test.helper import AsIsImporterMixin, ImportTestCase, PluginMixin -class LoggingTest(unittest.TestCase): - def test_logging_management(self): +class TestStrFormatLogger: + """Tests for the custom str-formatting logger.""" + + def test_logger_creation(self): l1 = log.getLogger("foo123") l2 = blog.getLogger("foo123") assert l1 == l2 @@ -35,17 +37,34 @@ def test_logging_management(self): l6 = blog.getLogger() assert l1 != l6 - def test_str_format_logging(self): - logger = blog.getLogger("baz123") - stream = StringIO() - handler = log.StreamHandler(stream) - - logger.addHandler(handler) - logger.propagate = False - - logger.warning("foo {} {bar}", "oof", bar="baz") - handler.flush() - assert stream.getvalue(), "foo oof baz" + @pytest.mark.parametrize( + "level", [log.DEBUG, log.INFO, log.WARNING, log.ERROR] + ) + @pytest.mark.parametrize( + "msg, args, kwargs, expected", + [ + ("foo {} bar {}", ("oof", "baz"), {}, "foo oof bar baz"), + ( + "foo {bar} baz {foo}", + (), + {"foo": "oof", "bar": "baz"}, + "foo baz baz oof", + ), + ("no args", (), {}, "no args"), + ("foo {} bar {baz}", ("oof",), {"baz": "baz"}, "foo oof bar baz"), + ], + ) + def test_str_format_logging( + self, level, msg, args, kwargs, expected, caplog + ): + logger = blog.getLogger("test_logger") + logger.setLevel(level) + + with caplog.at_level(level, logger="test_logger"): + logger.log(level, msg, *args, **kwargs) + + assert caplog.records, "No log records were captured" + assert str(caplog.records[0].msg) == expected class DummyModule(ModuleType):