Skip to content

Commit

Permalink
discover_plugins: allow loading of multiple plugins
Browse files Browse the repository at this point in the history
Change-Id: I973ab584f581a84f20109715e330c85893a32d6a
  • Loading branch information
mo-ki committed Nov 13, 2023
1 parent 07c3442 commit 09b92c4
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 36 deletions.
8 changes: 2 additions & 6 deletions cmk/base/plugins/server_side_calls/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@
def load_active_checks() -> tuple[Sequence[str], Mapping[str, ActiveCheckConfig]]:
loaded = discover_plugins(
"server_side_calls",
"active_check_",
ActiveCheckConfig,
{ActiveCheckConfig: "active_check_"},
raise_errors=cmk.utils.debug.enabled(),
)
# TODO:
# * see if we really need to return the errors. Maybe we can just either ignore or raise them.
# * deal with duplicate names.
return [str(e) for e in loaded.errors], {
plugin.name: plugin for plugin in loaded.plugins.values()
}
Expand All @@ -29,13 +27,11 @@ def load_active_checks() -> tuple[Sequence[str], Mapping[str, ActiveCheckConfig]
def load_special_agents() -> tuple[Sequence[str], Mapping[str, SpecialAgentConfig]]:
loaded = discover_plugins(
"server_side_calls",
"special_agent_",
SpecialAgentConfig,
{SpecialAgentConfig: "special_agent_"},
raise_errors=cmk.utils.debug.enabled(),
)
# TODO:
# * see if we really need to return the errors. Maybe we can just either ignore or raise them.
# * deal with duplicate names.
return [str(e) for e in loaded.errors], {
plugin.name: plugin for plugin in loaded.plugins.values()
}
33 changes: 20 additions & 13 deletions cmk/discover_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ class DiscoveredPlugins(Generic[_PluginType]):

def discover_plugins(
plugin_group: str,
name_prefix: str,
plugin_type: type[_PluginType],
*,
plugin_prefixes: Mapping[type[_PluginType], str],
raise_errors: bool,
) -> DiscoveredPlugins[_PluginType]:
"""Collect all plugins from well-known locations"""
Expand All @@ -71,7 +69,7 @@ def discover_plugins(
)
namespaces_by_priority = find_namespaces(modules, plugin_group, ls=_ls_defensive)

collector = Collector(plugin_type, name_prefix, raise_errors=raise_errors)
collector = Collector(plugin_prefixes, raise_errors=raise_errors)
for mod_name in namespaces_by_priority:
collector.add_from_module(mod_name, _import_optionally)

Expand Down Expand Up @@ -117,7 +115,7 @@ def _import_optionally(module_name: str, raise_errors: bool) -> ModuleType | Non
return importlib.import_module(module_name)
except ModuleNotFoundError:
return None # never choke upon empty/non-existing folders.
except Exception as _exc:
except Exception:
if raise_errors:
raise
return None
Expand All @@ -133,13 +131,18 @@ def _ls_defensive(path: str) -> Sequence[str]:
class Collector(Generic[_PluginType]):
def __init__(
self,
plugin_type: type[_PluginType],
name_prefix: str,
*,
plugin_prefixes: Mapping[type[_PluginType], str],
raise_errors: bool,
) -> None:
self.plugin_type: Final = plugin_type
self.name_prefix: Final = name_prefix
# Transform plugin types / prefixes to the data structure we
# need for this algorithm.
# We pass them differently to help the type inference along.
# Other approaches require even worse explicit type annotations
# on caller side.
self._prefix_to_types = tuple(
(prefix, tuple(t for t, p in plugin_prefixes.items() if p == prefix))
for prefix in set(plugin_prefixes.values())
)
self.raise_errors: Final = raise_errors

self.errors: list[Exception] = []
Expand Down Expand Up @@ -170,11 +173,15 @@ def _collect_module_plugins(
) -> None:
"""Dispatch valid and invalid well-known objects"""
for name, value in objects.items():
if not name.startswith(self.name_prefix):
continue
try:
plugin_types = next(
types for prefix, types in self._prefix_to_types if name.startswith(prefix)
)
except StopIteration:
continue # no match

location = PluginLocation(module_name, name)
if not isinstance(value, self.plugin_type):
if not isinstance(value, plugin_types):
self._handle_error(TypeError(f"{location}: {value!r}"))
continue

Expand Down
7 changes: 3 additions & 4 deletions cmk/gui/utils/rulespecs/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@

from collections.abc import Mapping, Sequence

from cmk.discover_plugins import discover_plugins, DiscoveredPlugins
from cmk.discover_plugins import discover_plugins
from cmk.rulesets.v1 import RuleSpec


def load_api_v1_rulespecs(raise_errors: bool) -> tuple[Sequence[str], Mapping[str, RuleSpec]]:
discovered_plugins: DiscoveredPlugins[RuleSpec] = discover_plugins(
discovered_plugins = discover_plugins(
"rulesets",
"rulespec_",
RuleSpec,
{RuleSpec: "rulespec_"},
raise_errors=raise_errors,
)
errors = [str(e) for e in discovered_plugins.errors]
Expand Down
44 changes: 31 additions & 13 deletions tests/unit/cmk/test_discover_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from collections.abc import Iterable
from dataclasses import dataclass
from types import ModuleType
from types import ModuleType, UnionType

import pytest

Expand Down Expand Up @@ -101,16 +101,16 @@ def test_find_namespaces_deduplicate_preserving_order() -> None:


@dataclass
class MyPluginBase:
class MyTestPlugin:
name: str


class MyTestPlugin(MyPluginBase):
pass
@dataclass
class MyOtherPlugin:
name: str


class MyOtherPlugin(MyPluginBase):
pass
AllMyPlugins: UnionType = MyTestPlugin | MyOtherPlugin


class TestCollector:
Expand All @@ -125,12 +125,14 @@ def _importer(self, module_name: str, raise_errors: bool) -> ModuleType | None:
return _make_module("my_other_type", [], my_plugin_2=MyOtherPlugin("herta"))
case "my_collision":
return _make_module("my_collision", [], my_plugin_3=MyTestPlugin("herta"))
case "your_module":
return _make_module("your_module", [], your_plugin_1=MyOtherPlugin("herbert"))

raise ValueError("this test seems broken")

def test_sipmle_ok_case(self) -> None:
"""Load a plugin"""
collector = Collector(MyTestPlugin, "my_", raise_errors=False)
collector = Collector({MyTestPlugin: "my_"}, raise_errors=False)
collector.add_from_module("my_module", self._importer)
assert not collector.errors
assert collector.plugins == {
Expand All @@ -139,36 +141,36 @@ def test_sipmle_ok_case(self) -> None:

def test_mising_ok(self) -> None:
"""Ignore missing modules"""
collector = Collector(MyTestPlugin, "my_", raise_errors=True)
collector = Collector({MyTestPlugin: "my_"}, raise_errors=True)
# missing is ok, even if raise_errors is true.
collector.add_from_module("nonexistant", self._importer)
assert not collector.errors
assert not collector.plugins

def test_unknown_name_ignored(self) -> None:
"""Do not load a plugin with name prefix missmatch"""
collector = Collector(MyTestPlugin, "your_", raise_errors=False)
collector = Collector({MyTestPlugin: "your_"}, raise_errors=False)
collector.add_from_module("my_module", self._importer)
assert not collector.errors
assert not collector.plugins

def test_wrong_type_raise(self) -> None:
"""Raise if a plugin has the wrong type"""
collector = Collector(MyTestPlugin, "my_", raise_errors=True)
collector = Collector({MyTestPlugin: "my_"}, raise_errors=True)

with pytest.raises(TypeError):
collector.add_from_module("my_other_type", self._importer)

def test_wrong_type_recorded(self) -> None:
"""Record the error if a plugin has the wrong type"""
collector = Collector(MyTestPlugin, "my_", raise_errors=False)
collector = Collector({MyTestPlugin: "my_"}, raise_errors=False)
collector.add_from_module("my_other_type", self._importer)
assert len(collector.errors) == 1
assert isinstance(collector.errors[0], TypeError)
assert not collector.plugins

def test_name_collision_same_type(self) -> None:
collector = Collector(MyTestPlugin, "my_", raise_errors=True)
collector = Collector({MyTestPlugin: "my_"}, raise_errors=True)
collector.add_from_module("my_module", self._importer)

with pytest.raises(ValueError, match="already defined"):
Expand All @@ -182,11 +184,27 @@ def test_name_collision_same_type(self) -> None:

def test_name_collision_different_type(self) -> None:
"""Plugins with same name but different type is ok"""
collector = Collector(MyPluginBase, "my_", raise_errors=False)
collector = Collector[MyOtherPlugin | MyTestPlugin](
{MyTestPlugin: "my_", MyOtherPlugin: "my_"}, raise_errors=False
)
collector.add_from_module("my_module", self._importer)
collector.add_from_module("my_other_type", self._importer)

assert not collector.errors
assert collector.plugins == {
PluginLocation("my_module", "my_plugin_1"): MyTestPlugin("herta"),
PluginLocation("my_other_type", "my_plugin_2"): MyOtherPlugin("herta"),
}

def test_different_prefixes(self) -> None:
collector = Collector[MyTestPlugin | MyOtherPlugin](
{MyTestPlugin: "my_", MyOtherPlugin: "your_"}, raise_errors=False
)
collector.add_from_module("my_module", self._importer)
collector.add_from_module("your_module", self._importer)

assert not collector.errors
assert collector.plugins == {
PluginLocation("my_module", "my_plugin_1"): MyTestPlugin("herta"),
PluginLocation("your_module", "your_plugin_1"): MyOtherPlugin("herbert"),
}

0 comments on commit 09b92c4

Please sign in to comment.