feat: Register plugins through entry points #87
Conversation
📝 WalkthroughWalkthroughThe changes refactor the plugin registry system to implement a singleton pattern with dual discovery mechanisms—one by package name and another by entry point—while introducing abstract properties and methods for concrete implementations. Test infrastructure includes example plugins and comprehensive test scenarios covering valid and invalid plugin configurations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Registry as PluginRegistryBase
participant ByName as Discovery:<br/>By Name
participant ByEntryPoint as Discovery:<br/>By Entry Point
participant Module as Module Loader
participant Validator as Validator
Client->>Registry: collect_plugins()
activate Registry
rect rgba(150, 180, 200, 0.2)
Note over Registry,Module: Name-based Discovery
Registry->>ByName: _collect_plugins_by_name()
activate ByName
ByName->>Module: import module<br/>(module_prefix + "*")
Module->>ByName: module loaded
ByName->>Validator: validate_plugin(module)
Validator->>ByName: ✓ valid
ByName->>Registry: plugins registered
deactivate ByName
end
rect rgba(150, 200, 150, 0.2)
Note over Registry,Validator: Entry Point Discovery
Registry->>ByEntryPoint: _collect_plugins_by_entry_point()
activate ByEntryPoint
ByEntryPoint->>Module: entry_points(entry_point)
Module->>ByEntryPoint: entry point objects
ByEntryPoint->>Module: load entry point → module
Module->>ByEntryPoint: module instance
ByEntryPoint->>Validator: validate_plugin(module)
Validator->>ByEntryPoint: ✓ valid
ByEntryPoint->>Registry: plugins registered
deactivate ByEntryPoint
end
deactivate Registry
Client->>Registry: get_plugin(name)
activate Registry
Registry->>Client: return TPlugin instance
deactivate Registry
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/snakemake_interface_common/plugin_registry/__init__.py (2)
195-200: Consider including more context in the import error message.The error message says "unable to import {ep.value}" but doesn't include the original exception message. Since the exception is chained (
from exc), the full traceback will be available, but theInvalidPluginExceptionmessage itself could be more informative.try: module = ep.load() except ImportError as exc: raise InvalidPluginException( - ep.name, f"unable to import {ep.value}" + ep.name, f"unable to import {ep.value}: {exc}" ) from exc
191-193: Silent duplicate skip prioritizes name-based discovery.Entry point plugins are silently skipped if a name-based plugin already exists. This is consistent with the documented behavior (name-based runs first), but users might not realize their entry point is being ignored. Consider logging a debug message when skipping duplicates for easier troubleshooting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pyproject.tomlis excluded by!pyproject.toml
📒 Files selected for processing (9)
src/snakemake_interface_common/plugin_registry/__init__.py(5 hunks)tests/__init__.py(1 hunks)tests/example_plugin.py(1 hunks)tests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.py(1 hunks)tests/plugins/invalid-object/snakemake_example_plugin_invalid_object/__init__.py(1 hunks)tests/plugins/missing-attr/snakemake_example_plugin_missing_attr/__init__.py(1 hunks)tests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py(1 hunks)tests/plugins/valid/snakemake_example_plugin_valid_2/__init__.py(1 hunks)tests/test_registry.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
Files:
tests/plugins/valid/snakemake_example_plugin_valid_2/__init__.pytests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.pytests/__init__.pytests/plugins/invalid-object/snakemake_example_plugin_invalid_object/__init__.pytests/plugins/missing-attr/snakemake_example_plugin_missing_attr/__init__.pysrc/snakemake_interface_common/plugin_registry/__init__.pytests/test_registry.pytests/example_plugin.pytests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py
🧬 Code graph analysis (5)
tests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.py (1)
tests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py (1)
ExampleSettings(9-10)
src/snakemake_interface_common/plugin_registry/__init__.py (3)
src/snakemake_interface_common/plugin_registry/plugin.py (2)
name(85-85)register_cli_args(134-198)src/snakemake_interface_common/plugin_registry/attribute_types.py (1)
AttributeType(17-31)src/snakemake_interface_common/exceptions.py (1)
InvalidPluginException(77-79)
tests/test_registry.py (4)
src/snakemake_interface_common/plugin_registry/plugin.py (1)
SettingsBase(41-50)src/snakemake_interface_common/exceptions.py (1)
InvalidPluginException(77-79)tests/example_plugin.py (5)
ExamplePlugin(18-34)ExamplePluginRegistry(37-76)new(39-43)name(25-26)settings_cls(33-34)src/snakemake_interface_common/plugin_registry/__init__.py (3)
get_plugin_type(147-159)get_plugin(117-132)get_registered_plugins(109-111)
tests/example_plugin.py (3)
src/snakemake_interface_common/plugin_registry/__init__.py (5)
PluginRegistryBase(34-269)module_prefix(87-88)entry_point(91-97)load_plugin(100-101)expected_attributes(104-105)src/snakemake_interface_common/plugin_registry/plugin.py (2)
PluginBase(82-360)SettingsBase(41-50)src/snakemake_interface_common/plugin_registry/attribute_types.py (3)
AttributeType(17-31)AttributeMode(6-8)AttributeKind(11-13)
tests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py (2)
src/snakemake_interface_common/plugin_registry/plugin.py (1)
SettingsBase(41-50)tests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.py (1)
ExampleSettings(6-7)
🔇 Additional comments (23)
tests/__init__.py (1)
1-1: LGTM!Standard Python package initialization with a clear comment explaining its purpose.
tests/plugins/valid/snakemake_example_plugin_valid_2/__init__.py (1)
1-3: LGTM!Valid test fixture that correctly provides the required
example_stringattribute without an optionalExampleSettingsclass, enabling testing of optional attribute handling.tests/plugins/missing-attr/snakemake_example_plugin_missing_attr/__init__.py (1)
1-1: LGTM!Correct test fixture that intentionally omits the required
example_stringattribute to validate error handling for missing attributes.tests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.py (1)
1-7: LGTM!Correct test fixture where
ExampleSettingsintentionally does not inherit fromSettingsBaseto validate class inheritance checking.tests/plugins/invalid-object/snakemake_example_plugin_invalid_object/__init__.py (1)
1-3: LGTM!Correct test fixture where
example_stringis intentionally set to an integer instead of a string to validate type checking.tests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py (1)
1-10: LGTM!Valid test fixture that correctly provides both the required
example_stringattribute and an optionalExampleSettingsclass that properly inherits fromSettingsBase.tests/test_registry.py (7)
19-33: LGTM!Well-structured helper function that correctly monkeypatches the entry points system for testing. The implementation properly wraps the entry points and maintains the
selectinterface.
36-47: LGTM!Comprehensive test of basic registry functionality including singleton behavior, plugin type derivation, and error handling for missing plugins.
50-99: LGTM!Thorough test of plugin discovery mechanisms covering both module-name-based and entry-point-based discovery. Correctly validates plugin attributes, settings class inheritance, and filtering of non-matching entry points.
101-111: LGTM!Correct test for missing required attribute validation. Properly verifies that an
InvalidPluginExceptionis raised with an informative error message.
114-124: LGTM!Correct test for object type validation. Properly verifies that an
InvalidPluginExceptionis raised when an attribute has an incorrect type.
127-138: LGTM!Correct test for class inheritance validation. Properly verifies that an
InvalidPluginExceptionis raised when a settings class doesn't inherit from the required base class.
141-178: LGTM!Comprehensive test of entry point validation covering both syntax errors (colon in module path) and import failures. Properly uses context managers for test isolation.
tests/example_plugin.py (5)
17-34: LGTM!Well-structured test plugin class that correctly implements the
PluginBaseinterface with appropriate property accessors for private fields and proper dataclass usage.
38-43: LGTM!The
new()method correctly bypasses the singleton pattern for test isolation by usingobject.__new__(cls)and explicitly calling__init__().
45-51: LGTM!Properties correctly implement the abstract registry interface with appropriate prefix and entry point values for testing.
53-62: LGTM!The
load_pluginmethod correctly constructs the plugin instance. Direct attribute access on line 55 is safe because this method is called after validation, which ensures required attributes exist.
64-76: LGTM!The
expected_attributesmethod correctly defines validation constraints, specifying thatExampleSettingsis an optional class attribute that must subclassSettingsBase, andexample_stringis a required string object.src/snakemake_interface_common/plugin_registry/__init__.py (5)
69-81: Singleton pattern implementation looks correct.The use of
_instanceat class level combined with thehasattr(self, "plugins")check in__init__properly ensures single initialization. The idempotent__init__is a good pattern for singletons.
85-106: Well-designed abstract interface.The abstract properties and methods are clearly defined. Making
entry_pointoptional with aNonedefault maintains backward compatibility for registries that don't use entry point discovery.
117-132: Improved error messaging inget_plugin.The error message now includes the pip-installable package name, making it easier for users to know what to install. This is a good UX improvement.
204-269: Registration and validation logic is well-implemented.The
register_pluginandvalidate_pluginmethods properly handle:
- Duplicate prevention
- Optional vs required attributes
- Class vs instance type checking with clear error messages
The docstrings are thorough with proper parameter and exception documentation.
228-229: I'm unable to complete the verification due to persistent repository access failures. Since I cannot execute shell scripts against the codebase, I cannot definitively verify whetheris_valid_plugin_package_nameis used elsewhere, overridden by subclasses, or if it's truly dead code.Based on the original review comment context alone:
- The method always returns
True(stub implementation)- It's not called within the shown file
- Its purpose (validating plugin package names) is not being performed
However, without accessing the full codebase, I cannot confirm:
- Whether it's called by external code or subclasses
- If it's part of a public API contract
- Whether there are legitimate reasons for its current stub implementation
The original review comment cannot be fully verified due to technical constraints.
| TypeVar, | ||
| Generic, | ||
| Optional, | ||
| Self, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Python version requirements in project configuration files
echo "=== Checking pyproject.toml ==="
cat pyproject.toml 2>/dev/null | head -50
echo -e "\n=== Checking setup.py ==="
cat setup.py 2>/dev/null | head -50
echo -e "\n=== Checking setup.cfg ==="
cat setup.cfg 2>/dev/null | head -50Repository: snakemake/snakemake-interface-common
Length of output: 216
🌐 Web query:
snakemake-interface-common Python version requirement pyproject.toml
💡 Result:
snakemake-interface-common requires Python >= 3.8. [1][2]
Sources:
- Bioconda recipe for snakemake-interface-common (lists "depends python: >=3.8"). [1]
- PyPI package page (wheels published for "py3"; source/binaries target Python 3). [2]
🌐 Web query:
snakemake-interface-common dependencies typing_extensions pypi
💡 Result:
Short answer: No — snakemake-interface-common on PyPI does not list typing_extensions as a dependency. See the PyPI project page and the Bioconda recipe (which lists its dependencies: argparse-dataclass, configargparse, packaging, python >=3.8). [1][2]
Sources:
- PyPI: snakemake-interface-common project page. [1]
- Bioconda recipe for snakemake-interface-common (dependency list). [2]
Self import breaks backward compatibility with Python 3.8–3.10.
The project requires Python >= 3.8, but Self (imported from typing on line 20) was only added in Python 3.11. Users on Python 3.8–3.10 will encounter an ImportError at runtime. Since typing_extensions is not listed as a project dependency, use either:
- A conditional import:
from typing_extensions import Selfwith a fallback for Python 3.11+, or - Add
typing_extensionsas a dependency and import from there for consistency across all Python versions.
🤖 Prompt for AI Agents
In src/snakemake_interface_common/plugin_registry/__init__.py around line 20,
importing Self from typing breaks compatibility with Python 3.8–3.10; replace it
with a compatibility import that uses typing_extensions when running on older
Pythons (or add typing_extensions as a dependency and import Self from there).
Update the top-level imports so the module imports Self from typing_extensions
for Python <3.11 (or falls back to typing for 3.11+), and add typing_extensions
to pyproject/requirements if you choose the dependency approach.
1 participant
Note: this is built on top of #86, that should be merged first.
This adds the ability to register plugins through the entry point mechanism.
Adds a new
entry_pointproperty toPluginRegistryBase. Child classes may override it to return a string, which is the entry point group name. For example, if the logger plugin registry hasentry_pointvalue"loggers", the followingpyproject.tomlcould be used to register a logger plugin:The plugin's name is
my-loggerand its classes or other required values will be imported frommy_logger.submodule(a standard dotted module name that can be nested to any level).Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.