feat: load builtin snakemake executor plugins #73
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe changes introduce a new method called Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ExecutorPluginRegistry
participant SnakemakeExecutors
Client->>ExecutorPluginRegistry: call collect_plugins()
ExecutorPluginRegistry->>ExecutorPluginRegistry: call superclass collect_plugins()
ExecutorPluginRegistry->>SnakemakeExecutors: try to import local, dryrun, touch
alt Import Successful
ExecutorPluginRegistry->>ExecutorPluginRegistry: iterate over executors
ExecutorPluginRegistry->>ExecutorPluginRegistry: register_plugin(executor_name, executor_module)
else Import Failed
ExecutorPluginRegistry->>Client: exit without adding plugins
end
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
🧹 Outside diff range and nitpick comments (1)
snakemake_interface_executor_plugins/registry/__init__.py (1)
61-74: LGTM with suggestions for improvementThe new
collect_pluginsmethod is a good addition that extends the functionality of theExecutorPluginRegistryclass. It successfully loads built-in Snakemake executor plugins when available. Here are some suggestions for improvement:
- Consider adding a check to ensure that the
register_pluginmethod is only called if the executor module has a__name__attribute.- It might be beneficial to add error handling around the
register_plugincalls to catch and log any unexpected issues during registration.- To prevent potential side effects from multiple calls, consider adding a flag to track whether the built-in plugins have already been loaded.
Here's a suggested implementation incorporating these improvements:
def collect_plugins(self): """Collect plugins and call register_plugin for each.""" super().collect_plugins() if hasattr(self, '_builtin_plugins_loaded'): return try: from snakemake.executors import local as local_executor from snakemake.executors import dryrun as dryrun_executor from snakemake.executors import touch as touch_executor except ImportError: # snakemake not present, proceed without adding these plugins return for executor in [local_executor, dryrun_executor, touch_executor]: if hasattr(executor, '__name__'): try: self.register_plugin(executor.__name__, executor) except Exception as e: # Log the error or handle it as appropriate for your use case print(f"Failed to register plugin {executor.__name__}: {str(e)}") self._builtin_plugins_loaded = TrueThis implementation adds checks for the
__name__attribute, includes error handling for theregister_plugincalls, and uses a flag to prevent multiple loadings of the built-in plugins.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- snakemake_interface_executor_plugins/registry/init.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
snakemake_interface_executor_plugins/registry/__init__.py (1)
Pattern
**/*.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.
This enables simpler code when loading the registry and wanting to also use Snakemake's internal executor plugins (local, dryrun, touch).
Summary by CodeRabbit
New Features
Bug Fixes
snakemakepackage is not available.