feat: add plugin factory classes #3
Conversation
|
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 8 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request introduces significant enhancements to the hivemind plugin management system. It adds a new plugin type enumeration, comprehensive plugin discovery and loading mechanisms, and abstract base classes for databases and protocols. The changes focus on improving plugin management flexibility, error handling, and providing a standardized interface for different types of plugins such as databases, network protocols, and agent protocols. Changes
Sequence DiagramsequenceDiagram
participant PM as PluginManager
participant EPs as EntryPoints
participant PF as PluginFactory
participant P as Plugin
PM->>EPs: find_plugins(type)
EPs-->>PM: Available plugins
PM->>PF: create(plugin_name)
PF->>P: Instantiate plugin
PF-->>PM: Plugin instance
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
hivemind_plugin_manager/__init__.py (3)
17-50: Potential concurrency issues withfind_plugins._errored
find_plugins._erroredis used to store errored entry points but is defined as a module-level attribute. In multi-threaded scenarios, simultaneous modifications to this shared list could lead to unexpected behavior. Consider storing errors locally within the function or within a thread-safe structure.
105-117: Forward references in type hints
The parametersbus: Optional[Union['FakeBus', 'MessageBusClient']]andhm_protocol: Optional['HiveMindListenerProtocol']are strings, which can trigger lint errors for undefined names. This is valid Python syntax for forward references, but some linters complain. Consider adding:+ from typing import TYPE_CHECKING + if TYPE_CHECKING: + from ovos_utils.fakebus import FakeBus + from ovos_bus_client import MessageBusClient + from hivemind_bus_client.listener import HiveMindListenerProtocolat the top of the file to address these warnings.
🧰 Tools
🪛 Ruff (0.8.2)
110-110: Undefined name
FakeBus(F821)
110-110: Undefined name
MessageBusClient(F821)
111-111: Undefined name
HiveMindListenerProtocol(F821)
148-159: Demonstration code block
Printing discovered plugins by type is a handy example for developers. Make sure to remove or guard such demo code in production to avoid unnecessary overhead.hivemind_plugin_manager/database.py (1)
165-167: Security consideration forpassword
Storing a plaintext password in this attribute may be acceptable in some contexts, but consider encrypting or hashing the password if security is a concern.hivemind_plugin_manager/protocols.py (1)
43-43: Optionalhm_protocol
As with the factories, referencing'HiveMindListenerProtocol'might trigger static analysis warnings. If these references are intended as a forward reference, consider adding a conditional import block as well.🧰 Tools
🪛 Ruff (0.8.2)
43-43: Undefined name
HiveMindListenerProtocol(F821)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
hivemind_plugin_manager/__init__.py(1 hunks)hivemind_plugin_manager/database.py(2 hunks)hivemind_plugin_manager/protocols.py(4 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
hivemind_plugin_manager/protocols.py
26-26: Undefined name ClientDatabase
(F821)
32-32: Undefined name HiveMindClientConnection
(F821)
43-43: Undefined name HiveMindListenerProtocol
(F821)
68-68: Undefined name HiveMindListenerProtocol
(F821)
hivemind_plugin_manager/__init__.py
110-110: Undefined name FakeBus
(F821)
110-110: Undefined name MessageBusClient
(F821)
111-111: Undefined name HiveMindListenerProtocol
(F821)
124-124: Undefined name HiveMindListenerProtocol
(F821)
137-137: Undefined name HiveMindListenerProtocol
(F821)
🔇 Additional comments (15)
hivemind_plugin_manager/__init__.py (7)
1-2: Imports look good
No issues found regarding the newly added imports.
10-14: Enum usage is appropriate
Using an enum.Enum subclass is a clean and self-documenting way to define plugin types. The string-based inheritance is helpful for clarity when referencing these enum values.
52-66: Backward compatibility for entry point loading
The fallback to pkg_resources is a reasonable approach for older environments. This pattern is recognized and generally reliable.
68-84: Graceful handling of missing plugins
load_plugin returns None if a plugin is not found. This is acceptable but be mindful that some callers may expect an exception. Logging a warning is helpful, but confirm that returning None won’t silently mask issues downstream.
86-103: DatabaseFactory is well-structured
The usage of KeyError when a plugin is unavailable is a clear, unambiguous way to inform the caller. The code also correctly distinguishes between AbstractRemoteDB and local AbstractDB subclasses.
119-130: NetworkProtocolFactory
No functional issues identified. The logic consistently parallels other factories, making it easy to maintain.
🧰 Tools
🪛 Ruff (0.8.2)
124-124: Undefined name HiveMindListenerProtocol
(F821)
132-146: BinaryDataHandlerProtocolFactory
Implementation follows the same pattern as other factories. Exception handling for unknown plugins is consistent.
🧰 Tools
🪛 Ruff (0.8.2)
137-137: Undefined name HiveMindListenerProtocol
(F821)
hivemind_plugin_manager/database.py (3)
157-157: New AbstractDB class
Using @dataclass with abc.ABC is a neat pattern to combine data structure initialization with mandatory method constraints.
266-276: AbstractRemoteDB extends AbstractDB
This subclass approach is clean for differentiating additional fields like host and port. The annotation of Optional[int] for port is appropriate for placeholders.
277-318: Enforcing the core abstract methods
The repeated abstract methods (add_item, search_by_value, __len__, __iter__) ensure consistency across DB implementations. Good practice for large plugin ecosystems.
hivemind_plugin_manager/protocols.py (5)
21-22: Preventing attribute errors
Returning a default NodeIdentity() when hm_protocol is absent safeguards against None dereferencing. Good defensive design.
26-28: Database property gracefully handles null protocol
Returning None for the database property if hm_protocol is not set avoids potential runtime errors.
🧰 Tools
🪛 Ruff (0.8.2)
26-26: Undefined name ClientDatabase
(F821)
33-34: Clients property fallback
Returning an empty dictionary if hm_protocol is missing is a safe default. Just be sure that calling code handles the case where no clients are present.
53-57: NetworkProtocol property access
agent_protocol is neatly encapsulated, returning None if hm_protocol is not set. This avoids repeated boilerplate checks.
68-75: BinaryDataHandlerProtocol’s agent linkage
The logic in __post_init__ ensures seamless integration if the hm_protocol is assigned later. This is a good approach when dealing with multiple interdependent protocols.
🧰 Tools
🪛 Ruff (0.8.2)
68-68: Undefined name HiveMindListenerProtocol
(F821)
Summary by CodeRabbit
Release Notes
New Features
Improvements
Technical Updates