From c34b2a00a4d25acf3947403777c8625646a78577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 23 Sep 2025 15:03:17 +0100 Subject: [PATCH 1/8] Fix plugin loading --- beets/plugins.py | 11 +++++++++-- docs/changelog.rst | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index d8d465183..c0dd12e5b 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -22,6 +22,7 @@ import re import sys from collections import defaultdict from functools import wraps +from importlib import import_module from pathlib import Path from types import GenericAlias from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar @@ -365,11 +366,11 @@ def _get_plugin(name: str) -> BeetsPlugin | None: """ try: try: - namespace = __import__(f"{PLUGIN_NAMESPACE}.{name}", None, None) + namespace = import_module(f"{PLUGIN_NAMESPACE}.{name}") except Exception as exc: raise PluginImportError(name) from exc - for obj in getattr(namespace, name).__dict__.values(): + for obj in namespace.__dict__.values(): if ( inspect.isclass(obj) and not isinstance( @@ -378,6 +379,12 @@ def _get_plugin(name: str) -> BeetsPlugin | None: and issubclass(obj, BeetsPlugin) and obj != BeetsPlugin and not inspect.isabstract(obj) + # Only consider this plugin's module or submodules to avoid + # conflicts when plugins import other BeetsPlugin classes + and ( + obj.__module__ == namespace.__name__ + or obj.__module__.startswith(f"{namespace.__name__}.") + ) ): return obj() diff --git a/docs/changelog.rst b/docs/changelog.rst index cbcfe73f0..52e935445 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -32,6 +32,8 @@ Bug fixes: extraartists field. - :doc:`plugins/spotify` Fixed an issue where candidate lookup would not find matches due to query escaping (single vs double quotes). +- :doc:`plugins/chroma` :doc:`plugins/bpsync` Fix plugin loading issue caused by + an import of another :class:`beets.plugins.BeetsPlugin` class. :bug:`6033` For packagers: From 7954671c737cbcd462f8b6dc22a062b52d260184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 23 Sep 2025 15:45:24 +0100 Subject: [PATCH 2/8] Mock DummyPlugin properly --- test/test_logging.py | 65 ++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/test/test_logging.py b/test/test_logging.py index aee0bd61b..b751dd70e 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -5,9 +5,10 @@ import sys import threading import unittest from io import StringIO +from types import ModuleType +from unittest.mock import patch import beets.logging as blog -import beetsplug from beets import plugins, ui from beets.test import _common, helper from beets.test.helper import AsIsImporterMixin, ImportTestCase, PluginMixin @@ -47,36 +48,46 @@ class LoggingTest(unittest.TestCase): assert stream.getvalue(), "foo oof baz" +class DummyModule(ModuleType): + class DummyPlugin(plugins.BeetsPlugin): + def __init__(self): + plugins.BeetsPlugin.__init__(self, "dummy") + self.import_stages = [self.import_stage] + self.register_listener("dummy_event", self.listener) + + def log_all(self, name): + self._log.debug("debug {}", name) + self._log.info("info {}", name) + self._log.warning("warning {}", name) + + def commands(self): + cmd = ui.Subcommand("dummy") + cmd.func = lambda _, __, ___: self.log_all("cmd") + return (cmd,) + + def import_stage(self, session, task): + self.log_all("import_stage") + + def listener(self): + self.log_all("listener") + + def __init__(self, *_, **__): + module_name = "beetsplug.dummy" + super().__init__(module_name) + self.DummyPlugin.__module__ = module_name + self.DummyPlugin = self.DummyPlugin + + class LoggingLevelTest(AsIsImporterMixin, PluginMixin, ImportTestCase): plugin = "dummy" - class DummyModule: - class DummyPlugin(plugins.BeetsPlugin): - def __init__(self): - plugins.BeetsPlugin.__init__(self, "dummy") - self.import_stages = [self.import_stage] - self.register_listener("dummy_event", self.listener) + @classmethod + def setUpClass(cls): + patcher = patch.dict(sys.modules, {"beetsplug.dummy": DummyModule()}) + patcher.start() + cls.addClassCleanup(patcher.stop) - def log_all(self, name): - self._log.debug("debug {}", name) - self._log.info("info {}", name) - self._log.warning("warning {}", name) - - def commands(self): - cmd = ui.Subcommand("dummy") - cmd.func = lambda _, __, ___: self.log_all("cmd") - return (cmd,) - - def import_stage(self, session, task): - self.log_all("import_stage") - - def listener(self): - self.log_all("listener") - - def setUp(self): - sys.modules["beetsplug.dummy"] = self.DummyModule - beetsplug.dummy = self.DummyModule - super().setUp() + super().setUpClass() def test_command_level0(self): self.config["verbose"] = 0 From 461bc049a058a233afe02c5ae3ba4a9571651e75 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Wed, 17 Sep 2025 11:42:07 +0200 Subject: [PATCH 3/8] Enhanced custom logger typing and logging tests --- beets/logging.py | 47 ++++++++++++++++++++++++++++++-------------- test/test_logging.py | 45 +++++++++++++++++++++++++++++------------- 2 files changed, 63 insertions(+), 29 deletions(-) diff --git a/beets/logging.py b/beets/logging.py index fd8b1962f..becfff86f 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -20,6 +20,9 @@ use {}-style formatting and can interpolate keywords arguments to the logging calls (`debug`, `info`, etc). """ +from __future__ import annotations + +import logging import threading from copy import copy from logging import ( @@ -34,6 +37,9 @@ from logging import ( NullHandler, StreamHandler, ) +from typing import TYPE_CHECKING, Any, Mapping, TypeVar + +from typing_extensions import ParamSpec __all__ = [ "DEBUG", @@ -49,8 +55,14 @@ __all__ = [ "getLogger", ] +if TYPE_CHECKING: + T = TypeVar("T") -def logsafe(val): + +P = ParamSpec("P") + + +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 +95,45 @@ class StrFormatLogger(Logger): """ class _LogMessage: - def __init__(self, msg, args, kwargs): + def __init__( + self, + msg: str, + args: logging._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: logging._ArgsType = (), + exc_info: logging._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, ) diff --git a/test/test_logging.py b/test/test_logging.py index b751dd70e..4a62e3a3b 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -3,10 +3,9 @@ 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 @@ -14,8 +13,10 @@ 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 +36,33 @@ class LoggingTest(unittest.TestCase): l6 = blog.getLogger() assert l1 != l6 - def test_str_format_logging(self): - logger = blog.getLogger("baz123") - stream = StringIO() - handler = log.StreamHandler(stream) + @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) - logger.addHandler(handler) - logger.propagate = False + with caplog.at_level(level, logger="test_logger"): + logger.log(level, msg, *args, **kwargs) - logger.warning("foo {} {bar}", "oof", bar="baz") - handler.flush() - assert stream.getvalue(), "foo oof baz" + assert str(caplog.records[0].msg) == expected class DummyModule(ModuleType): From f637e5efbb29c47a3d1800e5c9233ecdea74a644 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Fri, 19 Sep 2025 15:23:36 +0200 Subject: [PATCH 4/8] Added overload to getLogger function. Added changelog entry and added myself to codeowners file. --- .github/CODEOWNERS | 3 +++ beets/logging.py | 12 ++++++++---- docs/changelog.rst | 6 ++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 767509c9a..bb888d520 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 becfff86f..29357c0f0 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -35,9 +35,10 @@ from logging import ( Handler, Logger, NullHandler, + RootLogger, StreamHandler, ) -from typing import TYPE_CHECKING, Any, Mapping, TypeVar +from typing import TYPE_CHECKING, Any, Mapping, TypeVar, overload from typing_extensions import ParamSpec @@ -173,9 +174,12 @@ my_manager = copy(Logger.manager) 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/docs/changelog.rst b/docs/changelog.rst index 52e935445..38037955e 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) -------------------------- From b2fc007480f50278c44eb5e7c2ea13181822a0bf Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Fri, 19 Sep 2025 15:35:51 +0200 Subject: [PATCH 5/8] Fixed plugin typehints: use actual logger class. --- beetsplug/fetchart.py | 2 +- beetsplug/lyrics.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 54c065da4..37e7426f6 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -36,10 +36,10 @@ from beets.util.config import sanitize_pairs 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 f492ab3cc..d245d6a14 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -42,10 +42,9 @@ from beets.autotag.distance import string_dist 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.""" From caebf185f11b24ab73ccbf0699e5078bccef6bcf Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Fri, 19 Sep 2025 15:39:43 +0200 Subject: [PATCH 6/8] Removed unused ParamSpec and added a consistency check in the tests. --- beets/logging.py | 5 ----- test/test_logging.py | 1 + 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/beets/logging.py b/beets/logging.py index 29357c0f0..086a590a0 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -40,8 +40,6 @@ from logging import ( ) from typing import TYPE_CHECKING, Any, Mapping, TypeVar, overload -from typing_extensions import ParamSpec - __all__ = [ "DEBUG", "INFO", @@ -60,9 +58,6 @@ if TYPE_CHECKING: T = TypeVar("T") -P = ParamSpec("P") - - def _logsafe(val: T) -> str | T: """Coerce `bytes` to `str` to avoid crashes solely due to logging. diff --git a/test/test_logging.py b/test/test_logging.py index 4a62e3a3b..f4dda331e 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -62,6 +62,7 @@ class TestStrFormatLogger: 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 From 837295e2502a220fea0798f8352756d85627ba07 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Sun, 21 Sep 2025 22:27:24 +0200 Subject: [PATCH 7/8] Added typehints from typeshed and removed default argument. --- beets/logging.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/beets/logging.py b/beets/logging.py index 086a590a0..64d6c50b1 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -22,7 +22,6 @@ calls (`debug`, `info`, etc). from __future__ import annotations -import logging import threading from copy import copy from logging import ( @@ -40,6 +39,8 @@ from logging import ( ) from typing import TYPE_CHECKING, Any, Mapping, TypeVar, overload +from typing_extensions import TypeAlias + __all__ = [ "DEBUG", "INFO", @@ -56,6 +57,15 @@ __all__ = [ if TYPE_CHECKING: T = TypeVar("T") + from types import TracebackType + + # see https://github.com/python/typeshed/blob/main/stdlib/logging/__init__.pyi + _SysExcInfoType: TypeAlias = ( + tuple[type[BaseException], BaseException, TracebackType | None] + | tuple[None, None, None] + ) + _ExcInfoType: TypeAlias = None | bool | _SysExcInfoType | BaseException + _ArgsType: TypeAlias = tuple[object, ...] | Mapping[str, object] def _logsafe(val: T) -> str | T: @@ -94,7 +104,7 @@ class StrFormatLogger(Logger): def __init__( self, msg: str, - args: logging._ArgsType, + args: _ArgsType, kwargs: dict[str, Any], ): self.msg = msg @@ -110,8 +120,8 @@ class StrFormatLogger(Logger): self, level: int, msg: object, - args: logging._ArgsType = (), - exc_info: logging._ExcInfoType = None, + args: _ArgsType, + exc_info: _ExcInfoType = None, extra: Mapping[str, Any] | None = None, stack_info: bool = False, stacklevel: int = 1, From 89c2e10680b56873a07738b0a745205391df7425 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Sun, 21 Sep 2025 22:29:55 +0200 Subject: [PATCH 8/8] Removed typealias, worked locally with mypy but does seem to cause issues with the ci. Also python 3.9 requires unions here... --- beets/logging.py | 16 +++++++--------- test/test_logging.py | 1 + 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/beets/logging.py b/beets/logging.py index 64d6c50b1..3ed5e5a84 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -37,9 +37,7 @@ from logging import ( RootLogger, StreamHandler, ) -from typing import TYPE_CHECKING, Any, Mapping, TypeVar, overload - -from typing_extensions import TypeAlias +from typing import TYPE_CHECKING, Any, Mapping, TypeVar, Union, overload __all__ = [ "DEBUG", @@ -60,12 +58,12 @@ if TYPE_CHECKING: from types import TracebackType # see https://github.com/python/typeshed/blob/main/stdlib/logging/__init__.pyi - _SysExcInfoType: TypeAlias = ( - tuple[type[BaseException], BaseException, TracebackType | None] - | tuple[None, None, None] - ) - _ExcInfoType: TypeAlias = None | bool | _SysExcInfoType | BaseException - _ArgsType: TypeAlias = tuple[object, ...] | Mapping[str, object] + _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: T) -> str | T: diff --git a/test/test_logging.py b/test/test_logging.py index f4dda331e..48f9cbfd8 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -4,6 +4,7 @@ import logging as log import sys import threading from types import ModuleType +from unittest.mock import patch import pytest