feat: get_class factory methods #7
Conversation
WalkthroughThe pull request introduces a new Changes
Possibly related PRs
Suggested labels
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 (1)
hivemind_plugin_manager/__init__.py (1)
18-23: Consider more informative error handling or custom exceptions.
RaisingKeyErroris fine here, but it may be more clear to define a custom exception when a plugin is not found. This clarifies the issue for users and avoids confusion in code that might expect typical dictionary-based errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hivemind_plugin_manager/__init__.py(3 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
hivemind_plugin_manager/__init__.py
49-49: Undefined name FakeBus
(F821)
49-49: Undefined name MessageBusClient
(F821)
50-50: Undefined name HiveMindListenerProtocol
(F821)
67-67: Undefined name HiveMindListenerProtocol
(F821)
85-85: Undefined name HiveMindListenerProtocol
(F821)
🔇 Additional comments (8)
hivemind_plugin_manager/__init__.py (8)
2-2: No issues importing Type explicitly.
Great to see rigorous type annotations. This improves type safety and readability.
32-35: Confirm subfolder path usage for remote vs. local DB.
Ensure that remote DB classes truly ignore or gracefully handle local-related parameters (e.g., subfolder).
39-44: LGTM!
The get_class method is consistent with the DatabaseFactory approach, centralizing lookups and raising an error if the plugin is missing.
57-62: LGTM!
The get_class method for the NetworkProtocolFactory ensures a clean approach to plugin retrieval.
69-70: Double-check optional config usage.
The plugin invocation looks straightforward, but verify that each plugin indeed expects or safely ignores config and hm_protocol.
75-81: Solid approach to plugin lookups.
The method ensures a consistent retrieval pattern. The KeyError message is descriptive for missing plugins.
88-89: LGTM!
The usage of cls.get_class for the BinaryDataHandlerProtocol maintains consistency and reduces repeated logic.
52-53: Verify references to forward-declared bus types.
The bus parameter hints at 'FakeBus' and 'MessageBusClient' which might be undefined or not imported. Consider either adding stubs or real imports to improve clarity and satisfy type checkers.
✅ Verification successful
Bus types are properly imported and defined
The bus types FakeBus and MessageBusClient are correctly imported in the codebase:
from ovos_bus_client import MessageBusClientfrom ovos_utils.fakebus import FakeBus
These imports are present in hivemind_plugin_manager/protocols.py and the types are properly used in type hints throughout the codebase, including the file under review. The type hints are correctly defined using Union[FakeBus, MessageBusClient] which matches the usage in the reviewed code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Attempt to find references or definitions of FakeBus and MessageBusClient in the project:
rg 'class FakeBus'
rg 'class MessageBusClient'
Length of output: 50
Script:
#!/bin/bash
# Let's try a broader search for these bus types and their imports
rg -i "fakebus|messagebusclient" -A 2
# Also search for any bus-related type hints or annotations
ast-grep --pattern 'bus: $_'
# Look for bus-related imports
rg "^from.*bus.*import|^import.*bus"
Length of output: 1445
Summary by CodeRabbit
New Features
get_classmethod to plugin factories for centralized plugin retrievalRefactor