From 34615bffb6e848e33c1f35914634d5ace7d9aef9 Mon Sep 17 00:00:00 2001 From: "jinli.yl" Date: Fri, 21 Aug 2026 17:29:22 +0800 Subject: [PATCH] refactor: streamline plugin validation --- reme/plugin.py | 19 +++++++++++-------- reme/plugin_cli.py | 16 +++++----------- tests/unit/test_plugin.py | 9 +++++++-- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/reme/plugin.py b/reme/plugin.py index 8bd715ca..5b574961 100644 --- a/reme/plugin.py +++ b/reme/plugin.py @@ -17,7 +17,7 @@ from .components.base_component import ComponentMixin from .components.component_registry import ComponentRegistry, create_application_registry from .config import deep_merge_config, expand_env_vars from .entry_point import PLUGIN_ENTRY_POINT_GROUP, find_entry_points, load_entry_point, unique_entry_point -from .plugin_manifest import load_package_manifest +from .plugin_manifest import PluginManifest, load_package_manifest @dataclass(frozen=True) @@ -50,17 +50,19 @@ def _load_backend(target: str, *, plugin_name: str) -> type[ComponentMixin]: module_name, separator, attribute = target.partition(":") if not separator or not module_name or not attribute or ":" in attribute: raise ValueError(f"Plugin '{plugin_name}' has invalid backend target: {target!r}") - value: Any = import_module(module_name) - for part in attribute.split("."): - value = getattr(value, part) + try: + value: Any = import_module(module_name) + for part in attribute.split("."): + value = getattr(value, part) + except (AttributeError, ImportError) as exc: + raise ValueError(f"Plugin '{plugin_name}' cannot load backend '{target}': {exc}") from exc if not isinstance(value, type) or not issubclass(value, ComponentMixin): raise TypeError(f"Plugin '{plugin_name}' backend '{target}' is not a ComponentMixin class") return value -def _load_manifest_plugin(name: str, package: str) -> Plugin: - """Load ``plugin.yaml`` from an entry point's package.""" - manifest = load_package_manifest(package, plugin_name=name) +def _plugin_from_manifest(name: str, manifest: PluginManifest) -> Plugin: + """Convert a parsed manifest into one runtime plugin descriptor.""" backends = tuple( Backend(backend_name, _load_backend(target, plugin_name=name)) for backend_name, target in manifest.backends.items() @@ -76,7 +78,8 @@ def _load_plugin(name: str, entry: EntryPoint) -> Plugin: from .components.component_registry import R with R.preserve(allow_mutation=True): - return _load_manifest_plugin(name, entry.value) + manifest = load_package_manifest(entry.value, plugin_name=name) + return _plugin_from_manifest(name, manifest) plugin = load_entry_point(entry, invoke=True) if not isinstance(plugin, Plugin): raise TypeError(f"Plugin entry point '{name}' did not return reme.plugin.Plugin") diff --git a/reme/plugin_cli.py b/reme/plugin_cli.py index 16fd3963..ce06b7a2 100644 --- a/reme/plugin_cli.py +++ b/reme/plugin_cli.py @@ -217,14 +217,12 @@ def _uninstall_plugin(args: argparse.Namespace) -> int: return result -def _validate_plugins(plugins) -> None: +def _validate_plugins(manager) -> None: """Validate imports, registry ownership, and merged application schema.""" from .components.component_registry import create_application_registry from .config.config_parser import resolve_app_config - from .plugin import PluginManager from .schema.application_config import ApplicationConfig - manager = PluginManager(plugins) registry = create_application_registry() manager.register(registry) ApplicationConfig(**manager.merge_config(resolve_app_config(log_config=False))) @@ -234,13 +232,13 @@ def _validate_installed(name: str) -> list[str]: from .plugin import PluginManager manager = PluginManager.discover([name]) - _validate_plugins(manager.plugins) + _validate_plugins(manager) return [name] def _validate_local(path: Path) -> list[str]: from .components.component_registry import R - from .plugin import Backend, Plugin, _load_backend + from .plugin import PluginManager, _plugin_from_manifest project_file = path if path.name == "pyproject.toml" else path / "pyproject.toml" if not project_file.is_file(): @@ -266,12 +264,8 @@ def _validate_local(path: Path) -> list[str]: if not manifest_path.is_file(): raise FileNotFoundError(f"Plugin manifest not found: {manifest_path}") manifest = parse_plugin_manifest(manifest_path.read_text(encoding="utf-8"), plugin_name=name) - backends = tuple( - Backend(backend_name, _load_backend(target, plugin_name=name)) - for backend_name, target in manifest.backends.items() - ) - plugins.append(Plugin(name=name, backends=backends, config=manifest.application_defaults)) - _validate_plugins(plugins) + plugins.append(_plugin_from_manifest(name, manifest)) + _validate_plugins(PluginManager(plugins)) finally: sys.path.remove(str(source_root.resolve())) return [plugin.name for plugin in plugins] diff --git a/tests/unit/test_plugin.py b/tests/unit/test_plugin.py index 5f4b7742..a86aaa15 100644 --- a/tests/unit/test_plugin.py +++ b/tests/unit/test_plugin.py @@ -1,6 +1,6 @@ """Tests for installed plugin discovery and application-local registration.""" -# pylint: disable=missing-class-docstring,missing-function-docstring +# pylint: disable=missing-class-docstring,missing-function-docstring,protected-access from pathlib import Path @@ -11,7 +11,7 @@ from reme.components.base_component import BaseComponent, ComponentMixin from reme.components.component_registry import ComponentRegistry, R from reme.config.config_parser import _load_config from reme.enumeration import ComponentEnum -from reme.plugin import Backend, Plugin, PluginManager +from reme.plugin import Backend, Plugin, PluginManager, _load_backend from reme.plugin_manifest import parse_plugin_manifest @@ -185,6 +185,11 @@ def test_plugin_manifest_requires_application_defaults_mapping(): parse_plugin_manifest("application_defaults: []\n", plugin_name="example") +def test_plugin_manifest_reports_missing_backend_attribute(): + with pytest.raises(ValueError, match="cannot load backend.*MissingStep"): + _load_backend("reme.plugin:MissingStep", plugin_name="missing") + + def test_plugin_manager_rejects_multiple_entry_point_providers(monkeypatch): descriptor = Plugin(name="example") _set_entry_points(