feat: add plugin scaffolding #89
Conversation
|
Warning Rate limit exceeded@johanneskoester has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 1 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)
📝 Walkthrough""" WalkthroughThis change introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (snakedeploy)
participant scaffold_plugin
participant ScaffoldPluginX
participant FileSystem
User->>CLI (snakedeploy): snakedeploy scaffold-snakemake-plugin <plugin_type>
CLI (snakedeploy)->>scaffold_plugin: scaffold_plugin(plugin_type)
scaffold_plugin->>ScaffoldPluginX: Instantiate (based on plugin_type)
ScaffoldPluginX->>FileSystem: Read/modify pyproject.toml, render templates, setup files
FileSystem-->>ScaffoldPluginX: Files written and dependencies installed
ScaffoldPluginX-->>User: Scaffolded plugin project ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 20
🔭 Outside diff range comments (1)
snakedeploy/templates/plugins/conventional_prs.yml.j2 (1)
1-17: Update action version to v5.5.3The workflow is solidly configured:
- Correct use of
pull_request_targetand trigger types- Proper Jinja2 templating for
GITHUB_TOKENTo leverage the latest fixes and features, bump the action version:
- uses: amannn/[email protected] + uses: amannn/[email protected][v5.5.3 released June 28, 2024 – ensures up-to-date Conventional Commits enforcement][1]
🧹 Nitpick comments (8)
snakedeploy/version.py (1)
25-25: LGTM! Consider adding version constraints for the toml dependency.The addition of the
tomldependency is appropriate for the plugin scaffolding functionality. The format follows the existing pattern correctly.However, consider specifying a minimum version constraint to ensure compatibility with required features, especially since the
tomlpackage has had various versions with different capabilities.snakedeploy/templates/plugins/ci.yml.j2 (3)
17-17: Update to newer checkout action version.The
actions/checkout@v3action is outdated. Consider using@v4for better security and features.- uses: actions/checkout@v3 + uses: actions/checkout@v4
46-46: Update to newer checkout action version.Same issue as the linting job - update to
@v4.- - uses: actions/checkout@v3 + - uses: actions/checkout@v4
67-70: Coverage reporting could be enhanced.The current coverage setup runs coverage but doesn't upload results or create coverage reports that can be viewed in PRs. Consider adding coverage upload for better visibility.
snakedeploy/templates/plugins/scheduler-plugins/init.py (1)
24-24: Fix copy-paste error in comment.The comment incorrectly mentions "storage-plugin-name" instead of "scheduler-plugin-name".
- # SNAKEMAKE_<storage-plugin-name>_<param-name>, all upper case. + # SNAKEMAKE_<scheduler-plugin-name>_<param-name>, all upper case.snakedeploy/scaffold_plugins/common.py (2)
45-45: Consider making the GitHub repository URL configurable.The GitHub repository URL is hard-coded as a placeholder. Consider prompting the user or accepting it as a parameter.
Consider adding a parameter to the
handlemethod or prompting the user for their GitHub repository URL instead of using a hard-coded placeholder.
130-130: Document the removal of test init.py.The removal of
tests/__init__.pyshould be documented to explain why it's necessary.Add a comment explaining why the test
__init__.pyis being removed:+ # Remove __init__.py in tests directory as it's not needed for pytest discovery (tests_path / "__init__.py").unlink(missing_ok=True)snakedeploy/templates/plugins/storage-plugins/init.py (1)
117-119: Consider completing the postprocess_query documentation.The comment suggests this method can be removed if not needed, but it's already implemented to return the query unchanged.
Update the comment to reflect the default implementation:
- # If required, overwrite the method postprocess_query from StorageProviderBase - # in order to e.g. normalize the query or add information from the settings to it. - # Otherwise, remove this method as it will be inherited from the base class. + # This method can be overridden to normalize the query or add information from + # the settings. The default implementation returns the query unchanged. def postprocess_query(self, query: str) -> str: return query
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
snakedeploy/client.py(3 hunks)snakedeploy/scaffold_plugins/__init__.py(1 hunks)snakedeploy/scaffold_plugins/common.py(1 hunks)snakedeploy/scaffold_plugins/executor_plugins.py(1 hunks)snakedeploy/scaffold_plugins/report_plugins.py(1 hunks)snakedeploy/scaffold_plugins/scheduler_plugins.py(1 hunks)snakedeploy/scaffold_plugins/software_deployment_plugins.py(1 hunks)snakedeploy/scaffold_plugins/storage_plugins.py(1 hunks)snakedeploy/templates/plugins/ci.yml.j2(1 hunks)snakedeploy/templates/plugins/conventional_prs.yml.j2(1 hunks)snakedeploy/templates/plugins/executor-plugins/init.py(1 hunks)snakedeploy/templates/plugins/executor-plugins/tests.py.j2(1 hunks)snakedeploy/templates/plugins/release_please.yml.j2(1 hunks)snakedeploy/templates/plugins/report-plugins/init.py(1 hunks)snakedeploy/templates/plugins/report-plugins/tests.py(1 hunks)snakedeploy/templates/plugins/scheduler-plugins/init.py(1 hunks)snakedeploy/templates/plugins/scheduler-plugins/tests.py(1 hunks)snakedeploy/templates/plugins/setup.cfg.j2(1 hunks)snakedeploy/templates/plugins/software-deployment-plugins/init.py(1 hunks)snakedeploy/templates/plugins/software-deployment-plugins/tests.py(1 hunks)snakedeploy/templates/plugins/storage-plugins/init.py(1 hunks)snakedeploy/templates/plugins/storage-plugins/tests.py(1 hunks)snakedeploy/version.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:
snakedeploy/version.pysnakedeploy/templates/plugins/report-plugins/tests.pysnakedeploy/scaffold_plugins/scheduler_plugins.pysnakedeploy/templates/plugins/scheduler-plugins/tests.pysnakedeploy/scaffold_plugins/storage_plugins.pysnakedeploy/scaffold_plugins/report_plugins.pysnakedeploy/scaffold_plugins/__init__.pysnakedeploy/scaffold_plugins/executor_plugins.pysnakedeploy/scaffold_plugins/software_deployment_plugins.pysnakedeploy/templates/plugins/scheduler-plugins/init.pysnakedeploy/templates/plugins/report-plugins/init.pysnakedeploy/client.pysnakedeploy/templates/plugins/software-deployment-plugins/tests.pysnakedeploy/templates/plugins/executor-plugins/init.pysnakedeploy/templates/plugins/storage-plugins/tests.pysnakedeploy/templates/plugins/software-deployment-plugins/init.pysnakedeploy/templates/plugins/storage-plugins/init.pysnakedeploy/scaffold_plugins/common.py
🧬 Code Graph Analysis (5)
snakedeploy/scaffold_plugins/storage_plugins.py (5)
snakedeploy/scaffold_plugins/common.py (4)
ScaffoldPlugin(11-142)get_templates(13-15)get_plugin_type(18-18)include_snakemake_dev_dependency(24-24)snakedeploy/scaffold_plugins/executor_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/scheduler_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/software_deployment_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/report_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)
snakedeploy/scaffold_plugins/__init__.py (6)
snakedeploy/exceptions.py (1)
UserError(43-44)snakedeploy/scaffold_plugins/executor_plugins.py (1)
ScaffoldSnakemakeExecutorPlugin(7-20)snakedeploy/scaffold_plugins/report_plugins.py (1)
ScaffoldSnakemakeReportPlugin(7-20)snakedeploy/scaffold_plugins/scheduler_plugins.py (1)
ScaffoldSnakemakeSchedulerPlugin(7-20)snakedeploy/scaffold_plugins/software_deployment_plugins.py (1)
ScaffoldSnakemakeSoftwareDeploymentPlugin(7-20)snakedeploy/scaffold_plugins/storage_plugins.py (1)
ScaffoldSnakemakeStoragePlugin(7-20)
snakedeploy/scaffold_plugins/executor_plugins.py (5)
snakedeploy/scaffold_plugins/common.py (4)
ScaffoldPlugin(11-142)get_templates(13-15)get_plugin_type(18-18)include_snakemake_dev_dependency(24-24)snakedeploy/scaffold_plugins/scheduler_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/software_deployment_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/storage_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/report_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)
snakedeploy/client.py (1)
snakedeploy/scaffold_plugins/__init__.py (1)
scaffold_plugin(15-29)
snakedeploy/scaffold_plugins/common.py (6)
snakedeploy/exceptions.py (1)
UserError(43-44)snakedeploy/scaffold_plugins/executor_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/scheduler_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/software_deployment_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/storage_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/report_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)
🪛 Ruff (0.12.2)
snakedeploy/templates/plugins/scheduler-plugins/init.py
6-6: snakemake_interface_scheduler_plugins.interfaces.dag.DAGSchedulerInterface imported but unused
Remove unused import: snakemake_interface_scheduler_plugins.interfaces.dag.DAGSchedulerInterface
(F401)
snakedeploy/templates/plugins/executor-plugins/init.py
86-86: Found useless expression. Either assign it to a variable or remove it.
(B018)
88-88: Found useless expression. Either assign it to a variable or remove it.
(B018)
🪛 GitHub Actions: CI
snakedeploy/templates/plugins/scheduler-plugins/init.py
[error] 6-6: pyflakes: 'snakemake_interface_scheduler_plugins.interfaces.dag.DAGSchedulerInterface' imported but unused
snakedeploy/templates/plugins/report-plugins/init.py
[error] 10-10: pyflakes: 'snakemake_interface_common.exceptions.WorkflowError' imported but unused
snakedeploy/templates/plugins/executor-plugins/init.py
[error] 16-16: pyflakes: 'snakemake_interface_common.exceptions.WorkflowError' imported but unused
snakedeploy/templates/plugins/software-deployment-plugins/init.py
[error] 17-17: pyflakes: 'snakemake_interface_common.exceptions.WorkflowError' imported but unused
snakedeploy/templates/plugins/storage-plugins/init.py
[error] 4-4: pyflakes: 'snakemake_interface_storage_plugins.storage_provider.QueryType' imported but unused
[error] 22-22: pyflakes: 'snakemake_interface_common.exceptions.WorkflowError' imported but unused
🔇 Additional comments (26)
snakedeploy/templates/plugins/executor-plugins/tests.py.j2 (1)
1-17: LGTM! Well-structured test template.The test template follows best practices:
- Proper inheritance from Snakemake's test base class
- Correct use of
__test__ = Truefor pytest discovery- Appropriate Jinja2 templating with
{{plugin_name}}- Helpful comments referencing the base class documentation
- Proper typing and method signatures
The stubbed
get_executor_settingsmethod with ellipsis is appropriate for a template that users will customize.snakedeploy/templates/plugins/executor-plugins/init.py (3)
25-49: LGTM! Comprehensive ExecutorSettings template.The
ExecutorSettingsdataclass is well-structured with:
- Proper inheritance from
ExecutorSettingsBase- Detailed metadata for CLI integration
- Clear documentation for each metadata field
- Appropriate use of Optional typing and default values
The extensive comments provide excellent guidance for plugin developers.
54-78: LGTM! Well-configured CommonSettings.The
CommonSettingsconfiguration is appropriate for a remote executor plugin with sensible defaults:
non_local_exec=Truefor remote executionimplies_no_shared_fs=Truefor cloud/cluster scenarios- Proper job deployment and environment variable handling
- Reasonable status check configuration
106-148: LGTM! Comprehensive executor method templates.The executor methods provide excellent scaffolding:
run_jobwith clear guidance on job submission and reportingcheck_active_jobswith detailed async implementation guidance including rate limitingcancel_jobsfor proper cleanup handling- Extensive comments explaining the interface requirements
The ellipsis placeholders are appropriate for template stubs that users will implement.
snakedeploy/templates/plugins/setup.cfg.j2 (1)
1-7: LGTM! Standard flake8 configuration compatible with Black.The configuration follows best practices:
- Line length of 88 matches Black's default formatting
- E203 ignore is necessary to avoid conflicts with Black's whitespace handling
- Clear comments explain the reasoning behind each setting
This provides a good foundation for consistent code style in scaffolded plugins.
snakedeploy/scaffold_plugins/executor_plugins.py (1)
7-21: Implementation follows established pattern correctly.The class correctly implements the
ScaffoldPlugininterface with appropriate settings for executor plugins (returnsTruefor Snakemake dev dependency, which is consistent with report plugins that also need Snakemake for testing).snakedeploy/templates/plugins/ci.yml.j2 (1)
23-35: Pixi usage is limited to scaffolding—no changes neededThe
pixicommands insnakedeploy/scaffold_plugins/common.pyare solely part of the project‐generation step and don’t affect runtime dependency handling. The CI template correctly relies on Poetry to install and manage project dependencies. You can safely ignore this comment.Likely an incorrect or invalid review comment.
snakedeploy/templates/plugins/scheduler-plugins/tests.py (1)
7-21: Test template implementation is correct.The test class properly inherits from
TestSchedulerBaseand implements the required abstract methods as placeholders. The__test__ = Trueattribute ensures test discovery, and the method signatures match the expected interface.snakedeploy/templates/plugins/report-plugins/tests.py (1)
8-17: Report test template is well-structured.The test class correctly inherits from
TestReportBaseand uses appropriate template variables. The{{plugin_name}}placeholder will be properly substituted during scaffolding.snakedeploy/scaffold_plugins/__init__.py (1)
15-30: Plugin dispatcher implementation is well-designed.The function provides a clean interface for plugin scaffolding with proper error handling and follows the established pattern. All supported plugin types are correctly mapped to their respective scaffold classes.
snakedeploy/scaffold_plugins/storage_plugins.py (1)
7-21: LGTM! Storage plugin scaffold implementation is correct.The implementation properly inherits from
ScaffoldPluginand correctly implements all required abstract methods. The template paths follow the established convention, and settinginclude_snakemake_dev_dependency()toFalseis consistent with scheduler and software-deployment plugins.snakedeploy/scaffold_plugins/report_plugins.py (1)
7-21: LGTM! Report plugin scaffold implementation is correct.The implementation properly inherits from
ScaffoldPluginand correctly implements all required abstract methods. The template paths follow the established convention, and settinginclude_snakemake_dev_dependency()toTrueis appropriate for report plugins that likely need snakemake during development.snakedeploy/scaffold_plugins/scheduler_plugins.py (1)
7-21: LGTM! Scheduler plugin scaffold implementation is correct.The implementation properly inherits from
ScaffoldPluginand correctly implements all required abstract methods. The template paths follow the established convention, and settinginclude_snakemake_dev_dependency()toFalseis appropriate for scheduler plugins.snakedeploy/scaffold_plugins/software_deployment_plugins.py (1)
7-21: LGTM! Software deployment plugin scaffold implementation is correct.The implementation properly inherits from
ScaffoldPluginand correctly implements all required abstract methods. The template paths follow the established convention, and settinginclude_snakemake_dev_dependency()toFalseis appropriate for software deployment plugins.snakedeploy/client.py (3)
17-17: LGTM! Import statement is correct.The import of
scaffold_pluginfromsnakedeploy.scaffold_pluginsis properly placed and necessary for the new scaffolding functionality.
226-240: LGTM! Subcommand parser is properly configured.The new
scaffold-snakemake-pluginsubcommand is correctly set up with appropriate description and plugin type choices that match the supported plugin types in the scaffolding framework.
313-314: LGTM! Subcommand handling is correct.The handling of the
scaffold-snakemake-pluginsubcommand properly calls thescaffold_pluginfunction with the provided plugin type argument.snakedeploy/templates/plugins/release_please.yml.j2 (2)
1-10: LGTM: Workflow structure and configuration are well-designed.The trigger configuration and environment setup follow GitHub Actions best practices for automated releases.
23-45: LGTM: Publish job follows current best practices.The job properly uses conditional execution, modern tooling (pixi), build verification, and secure PyPI publishing with token authentication.
snakedeploy/templates/plugins/report-plugins/init.py (1)
48-61: LGTM: Well-structured template with comprehensive documentation.The Reporter class provides clear guidance for implementation with proper inheritance and method stubs. The documentation explains the purpose and constraints effectively.
snakedeploy/templates/plugins/software-deployment-plugins/tests.py (1)
18-44: LGTM: Well-designed test class template with clear guidance.The test class provides comprehensive documentation for each method and proper configuration options. The structure follows testing best practices for plugin development.
snakedeploy/templates/plugins/storage-plugins/tests.py (1)
7-37: LGTM: Comprehensive test template with flexible capability configuration.The test class provides excellent configurability through capability flags and clear method documentation. The design accommodates various storage plugin types effectively.
snakedeploy/templates/plugins/scheduler-plugins/init.py (1)
46-86: LGTM: Comprehensive scheduler template with excellent documentation.The Scheduler class provides clear guidance for implementation with detailed comments explaining the purpose and parameters of each method. The lifecycle methods cover all essential scheduler functionality.
snakedeploy/scaffold_plugins/common.py (1)
11-28: LGTM!The abstract methods are well-defined with appropriate type hints and follow Python ABC conventions.
snakedeploy/templates/plugins/storage-plugins/init.py (1)
36-61: LGTM! Well-documented settings class.The
StorageProviderSettingsdataclass is well-structured with comprehensive documentation about metadata fields and their usage.snakedeploy/templates/plugins/software-deployment-plugins/init.py (1)
71-103: LGTM! Well-documented EnvSpec class.The
EnvSpecclass is well-documented with clear explanations about path handling and attribute requirements.
snakedeploy/templates/plugins/software-deployment-plugins/init.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
snakedeploy/templates/plugins/executor-plugins/init.py (2)
16-16: Remove unused import or provide usage example.The
WorkflowErrorimport is flagged as unused by the pipeline. Since this is a template file, either:
- Remove the import and the
# noqacomment- Add a usage example in the comments to demonstrate when/how to use it
-from snakemake_interface_common.exceptions import WorkflowError # noqa +# from snakemake_interface_common.exceptions import WorkflowError +# Use WorkflowError for non-job errors that should be reported to the user
84-88: Fix useless expressions in post_init method.The static analysis correctly flags lines 86 and 88 as useless expressions. These are likely meant as examples but should be properly commented or assigned to variables.
def __post_init__(self): - # access workflow - self.workflow - # access executor specific settings - self.workflow.executor_settings + # access workflow + # workflow = self.workflow + # access executor specific settings + # settings = self.workflow.executor_settingssnakedeploy/templates/plugins/storage-plugins/init.py (2)
9-9: Remove unused import causing pipeline failure.The
QueryTypeimport is unused and causing pipeline failures.Apply this diff to remove the unused import:
from snakemake_interface_storage_plugins.storage_provider import ( # noqa StorageProviderBase, StorageQueryValidationResult, ExampleQuery, Operation, - QueryType, )
22-22: Remove unused import causing pipeline failure.The
WorkflowErrorimport is unused and causing pipeline failures. Since this is a template file, convert it to a comment showing how to import when needed.Apply this diff to fix the unused import:
-# Raise errors that will not be handled within this plugin but thrown upwards to -# Snakemake and the user as WorkflowError. -from snakemake_interface_common.exceptions import WorkflowError # noqa +# Import WorkflowError when you need to raise errors that will be handled by Snakemake: +# from snakemake_interface_common.exceptions import WorkflowErrorsnakedeploy/templates/plugins/software-deployment-plugins/init.py (4)
15-17: Address the unused import flagged by CI.The
WorkflowErrorimport is flagged by the CI pipeline as unused. Since this is a template file, either provide an example usage or convert it to a commented import with usage instructions.
21-29: Fix copy-paste errors in documentation.The comments incorrectly refer to "storage plugin" instead of "software deployment plugin" in multiple places.
38-38: Fix environment variable naming documentation.The comment incorrectly references "storage-plugin-name" instead of "software-deployment-plugin-name".
67-67: Complete the common_settings configuration.The
providesfield is set to ellipsis (...) which needs to be replaced with an actual string value indicating the environment kind.
🧹 Nitpick comments (24)
snakedeploy/templates/plugins/storage-plugins/init.py (16)
52-55: Replace ellipsis placeholders with proper examples in template.The ellipsis (
...) placeholders forparse_funcandunparse_funcshould be replaced withNoneor example functions to provide clearer guidance to users.Apply this diff to improve the template:
# Optionally specify a function that parses the value given by the user. # This is useful to create complex types from the user input. - "parse_func": ..., + "parse_func": None, # e.g., lambda x: int(x) # If a parse_func is specified, you also have to specify an unparse_func # that converts the parsed value back to a string. - "unparse_func": ..., + "unparse_func": None, # e.g., lambda x: str(x)
83-86: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
@classmethod def example_queries(cls) -> List[ExampleQuery]: """Return an example queries with description for this storage provider (at least one).""" - ... + # TODO: Replace with actual example queries for your storage provider + # return [ExampleQuery(query="my-storage://path/to/file", description="Example query")] + raise NotImplementedError("Implement example_queries method")
88-95: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
def rate_limiter_key(self, query: str, operation: Operation) -> Any: """Return a key for identifying a rate limiter given a query and an operation. This is used to identify a rate limiter for the query. E.g. for a storage provider like http that would be the host name. For s3 it might be just the endpoint URL. """ - ... + # TODO: Return appropriate rate limiter key for your storage provider + # Example: return urlparse(query).netloc for HTTP-based storage + raise NotImplementedError("Implement rate_limiter_key method")
97-100: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
def default_max_requests_per_second(self) -> float: """Return the default maximum number of requests per second for this storage provider.""" - ... + # TODO: Return appropriate rate limit for your storage provider + # Example: return 1.0 for conservative rate limiting + raise NotImplementedError("Implement default_max_requests_per_second method")
102-104: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
def use_rate_limiter(self) -> bool: """Return False if no rate limiting is needed for this provider.""" - ... + # TODO: Return True if rate limiting is needed, False otherwise + # Example: return True for most network-based storage providers + raise NotImplementedError("Implement use_rate_limiter method")
106-112: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
@classmethod def is_valid_query(cls, query: str) -> StorageQueryValidationResult: """Return whether the given query is valid for this storage provider.""" # Ensure that also queries containing wildcards (e.g. {sample}) are accepted # and considered valid. The wildcards will be resolved before the storage # object is actually used. - ... + # TODO: Implement query validation logic for your storage provider + # Example: return StorageQueryValidationResult(valid=query.startswith("my-storage://")) + raise NotImplementedError("Implement is_valid_query method")
163-165: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
def local_suffix(self) -> str: """Return a unique suffix for the local path, determined from self.query.""" - ... + # TODO: Generate unique local suffix from query + # Example: return hashlib.md5(self.query.encode()).hexdigest() + raise NotImplementedError("Implement local_suffix method")
167-171: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
def cleanup(self): """Perform local cleanup of any remainders of the storage object.""" # self.local_path() should not be removed, as this is taken care of by # Snakemake. - ... + # TODO: Implement cleanup logic for your storage provider + # Example: close connections, clean temporary files, etc. + pass # Default: no cleanup needed
176-179: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
@retry_decorator def exists(self) -> bool: # return True if the object exists - ... + # TODO: Check if object exists in your storage + # Example: return requests.head(self.query).status_code == 200 + raise NotImplementedError("Implement exists method")
181-184: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
@retry_decorator def mtime(self) -> float: # return the modification time - ... + # TODO: Return modification time as Unix timestamp + # Example: return os.path.getmtime(self.local_path()) + raise NotImplementedError("Implement mtime method")
186-189: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
@retry_decorator def size(self) -> int: # return the size in bytes - ... + # TODO: Return object size in bytes + # Example: return os.path.getsize(self.local_path()) + raise NotImplementedError("Implement size method")
191-200: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
@retry_decorator def local_footprint(self) -> int: # Local footprint is the size of the object on the local disk. # For directories, this should return the recursive sum of the # directory file sizes. # If the storage provider supports ondemand eligibility (see retrieve_object() # below), this should return 0 if the object is not downloaded but e.g. # mounted upon retrieval. # If this method is not overwritten here, it defaults to self.size(). - ... + # TODO: Return local disk footprint in bytes + # Default implementation calls self.size() - override if needed + return self.size()
202-215: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
@retry_decorator def retrieve_object(self): # Ensure that the object is accessible locally under self.local_path() # Optionally, this can make use of the attribute self.is_ondemand_eligible, # which indicates that the object could be retrieved on demand, # e.g. by only symlinking or mounting it from whatever network storage this # plugin provides. For example, objects with self.is_ondemand_eligible == True # could mount the object via fuse instead of downloading it. # The job can then transparently access only the parts that matter to it # without having to wait for the full download. # On demand eligibility is calculated via Snakemake's access pattern annotation. # If no access pattern is annotated by the workflow developers, # self.is_ondemand_eligible is by default set to False. - ... + # TODO: Download/retrieve object to self.local_path() + # Example: urllib.request.urlretrieve(self.query, self.local_path()) + raise NotImplementedError("Implement retrieve_object method")
220-224: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
@retry_decorator def store_object(self): # Ensure that the object is stored at the location specified by # self.local_path(). - ... + # TODO: Upload/store object from self.local_path() to storage + # Example: upload_file(self.local_path(), self.query) + raise NotImplementedError("Implement store_object method")
226-229: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
@retry_decorator def remove(self): # Remove the object from the storage. - ... + # TODO: Delete object from storage + # Example: requests.delete(self.query) + raise NotImplementedError("Implement remove method")
234-241: Replace ellipsis with proper implementation stub.Template methods should provide clearer guidance than just ellipsis.
Apply this diff to improve the template:
@retry_decorator def list_candidate_matches(self) -> Iterable[str]: """Return a list of candidate matches in the storage for the query.""" # This is used by glob_wildcards() to find matches for wildcards in the query. # The method has to return concretized queries without any remaining wildcards. # Use snakemake_executor_plugins.io.get_constant_prefix(self.query) to get the # prefix of the query before the first wildcard. - ... + # TODO: Return list of concrete queries matching the wildcard pattern + # Example: return list_files_matching_pattern(self.query) + raise NotImplementedError("Implement list_candidate_matches method")snakedeploy/templates/plugins/software-deployment-plugins/init.py (8)
126-126: Replace ellipsis with proper implementation guidance.The
checkmethod contains ellipsis instead of implementation guidance. Provide a TODO comment or example.- ... + # TODO: Implement checks for required software availability + # Example: check if container runtime or module command is available + pass
130-130: Replace ellipsis with proper implementation guidance.The
decorate_shellcmdmethod contains ellipsis instead of implementation guidance.- ... + # TODO: Implement shell command decoration to run within the environment + # Example: return f"module load {self.spec.module_name} && {cmd}" + return cmd
137-137: Replace ellipsis with proper implementation guidance.The
record_hashmethod contains ellipsis instead of implementation guidance.- hash_object.update(...) + # TODO: Update hash with environment-specific data + # Example: hash_object.update(str(self.spec.env_name).encode()) + pass
147-147: Replace ellipsis with proper implementation guidance.The
report_softwaremethod contains ellipsis instead of implementation guidance.- ... + # TODO: Return list of SoftwareReport objects describing contained software + # Example: return [SoftwareReport(name="conda", version="4.10.0")] + return []
161-161: Replace ellipsis with proper implementation guidance.The
deploymethod contains ellipsis instead of implementation guidance.- ... + # TODO: Implement environment deployment logic + # Use self.run_cmd() for shell commands to maintain parent environment context + pass
170-170: Replace ellipsis with proper implementation guidance.The
is_deployment_path_portablemethod contains ellipsis instead of implementation guidance.- ... + # TODO: Return True if deployment can be moved, False otherwise + # Example: return False for conda environments (hardcoded RPATHs) + return True
176-176: Replace ellipsis with proper implementation guidance.The
removemethod contains ellipsis instead of implementation guidance.- ... + # TODO: Implement cleanup logic for deployed environment + # Example: shutil.rmtree(self.deployment_path) + pass
186-186: Replace ellipsis with proper implementation guidance.The
archivemethod contains ellipsis instead of implementation guidance.- ... + # TODO: Implement environment archiving logic + # Use self.run_cmd() for shell commands to maintain parent environment context + pass
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
snakedeploy/templates/plugins/executor-plugins/init.py(1 hunks)snakedeploy/templates/plugins/report-plugins/init.py(1 hunks)snakedeploy/templates/plugins/scheduler-plugins/init.py(1 hunks)snakedeploy/templates/plugins/software-deployment-plugins/init.py(1 hunks)snakedeploy/templates/plugins/storage-plugins/init.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- snakedeploy/templates/plugins/report-plugins/init.py
- snakedeploy/templates/plugins/scheduler-plugins/init.py
🧰 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:
snakedeploy/templates/plugins/storage-plugins/init.pysnakedeploy/templates/plugins/software-deployment-plugins/init.pysnakedeploy/templates/plugins/executor-plugins/init.py
🪛 GitHub Actions: CI
snakedeploy/templates/plugins/storage-plugins/init.py
[error] 4-4: ESLint: 'snakemake_interface_storage_plugins.storage_provider.QueryType' imported but unused.
[error] 22-22: ESLint: 'snakemake_interface_common.exceptions.WorkflowError' imported but unused.
snakedeploy/templates/plugins/software-deployment-plugins/init.py
[error] 17-17: ESLint: 'snakemake_interface_common.exceptions.WorkflowError' imported but unused.
snakedeploy/templates/plugins/executor-plugins/init.py
[error] 16-16: ESLint: 'snakemake_interface_common.exceptions.WorkflowError' imported but unused.
🪛 Ruff (0.12.2)
snakedeploy/templates/plugins/executor-plugins/init.py
86-86: Found useless expression. Either assign it to a variable or remove it.
(B018)
88-88: Found useless expression. Either assign it to a variable or remove it.
(B018)
🔇 Additional comments (3)
snakedeploy/templates/plugins/executor-plugins/init.py (3)
25-49: LGTM! Well-structured template class.The ExecutorSettings class provides a comprehensive example with detailed metadata options and clear documentation for template users.
54-78: LGTM! Appropriate settings for remote executor template.The common settings are correctly configured for remote execution with clear explanatory comments for each option.
106-148: LGTM! Well-documented template methods.The stub implementations provide comprehensive documentation and follow the correct interface patterns for executor plugins.
|
Hi @johanneskoester could snakemake/poetry-snakemake-plugin#41 be included in this? So logger plugins can be scaffolded. I can move those changes here. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
snakedeploy/providers.py (1)
2-2: Remove the unused import.The
copy_treeimport fromdistutils.dir_utilis no longer used after switching toshutil.copytree. Please remove this unused import.-from distutils.dir_util import copy_tree
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pixi.lockis excluded by!**/*.lockpyproject.tomlis excluded by!pyproject.toml
📒 Files selected for processing (12)
.gitattributes(1 hunks).gitignore(1 hunks).python-version(1 hunks)docs/conf.py(1 hunks)main.py(1 hunks)setup.cfg(0 hunks)setup.py(0 hunks)snakedeploy/conda.py(1 hunks)snakedeploy/conda_version.py(2 hunks)snakedeploy/providers.py(2 hunks)snakedeploy/version.py(0 hunks)versioneer.py(2 hunks)
💤 Files with no reviewable changes (3)
- setup.cfg
- snakedeploy/version.py
- setup.py
✅ Files skipped from review due to trivial changes (8)
- .python-version
- .gitattributes
- main.py
- docs/conf.py
- snakedeploy/conda.py
- .gitignore
- versioneer.py
- snakedeploy/conda_version.py
🧰 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:
snakedeploy/providers.py
🔇 Additional comments (1)
snakedeploy/providers.py (1)
3-4: Good modernization to use shutil instead of deprecated distutils.The switch from
distutils.dir_util.copy_treetoshutil.copytreeis a good modernization sincedistutilsis deprecated and will be removed in future Python versions.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
docs/conf.py (1)
1-220: Fix code formatting issues.The CI pipeline indicates that this file needs formatting. Run
ruff format docs/conf.pyto fix the formatting issues.
🧹 Nitpick comments (6)
docs/requirements.txt (1)
4-4: Consider pinning docutils version for stability.Removing the version constraint from
docutilscould lead to compatibility issues if a breaking change is introduced in future versions. Consider using a flexible version constraint likedocutils>=0.12,<1.0to allow updates while preventing major version changes.tests/test_client.sh (1)
89-91: Clean up temporary test directories.The test creates directories in
/tmp/snakemake-$plugin_type-plugin-testbut doesn't clean them up. This could lead to disk space issues in CI environments.Add cleanup after the test loop:
for plugin_type in executor storage report software-deployment do pixi init --format pyproject /tmp/snakemake-$plugin_type-plugin-test runTest 0 $output snakedeploy scaffold-snakemake-plugin $plugin_type /tmp/snakemake-$plugin_type-plugin-test done + +# Clean up test directories +for plugin_type in executor storage report software-deployment +do + rm -rf /tmp/snakemake-$plugin_type-plugin-test +done.readthedocs.yaml (1)
6-6: Consider pinning the pixi installer version for reproducibility.Downloading and executing a script from the internet poses potential security risks. Consider using a specific version or commit hash of the installer script.
- - curl -fsSL https://pixi.sh/install.sh | sh + - curl -fsSL https://pixi.sh/install.sh | bash -s -- --version 0.35.0.github/workflows/main.yml (1)
23-35: Consider testing across multiple Python versions.The refactoring removed the Python version matrix, which means the code is no longer tested across different Python versions. This could lead to compatibility issues going unnoticed.
Consider adding a matrix strategy if pixi supports specifying Python versions, or document the minimum supported Python version clearly.
docs/conf.py (2)
36-36: Remove commented out code.Instead of keeping commented out code, remove it entirely to maintain code cleanliness.
-# Snakmake theme (made by SciAni). -#html_css_files = ["theme.css"]
50-55: Consider adding version information to documentation.The previous configuration included logic to extract version information from the
snakedeploypackage. Without this, the documentation won't show version numbers, which could be confusing for users trying to match documentation to their installed version.Consider adding back version detection:
# The version info for the project you're documenting import snakedeploy version = snakedeploy.__version__ release = version
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/logo-snake.svgis excluded by!**/*.svgpyproject.tomlis excluded by!pyproject.toml
📒 Files selected for processing (17)
.github/workflows/main.yml(1 hunks).gitignore(1 hunks).readthedocs.yaml(1 hunks)docs/_static/theme.css(0 hunks)docs/conf.py(5 hunks)docs/getting_started/installation.rst(2 hunks)docs/index.rst(2 hunks)docs/requirements.txt(1 hunks)docs/snakemake_developers/scaffold_snakemake_plugins.rst(1 hunks)docs/workflow_developers/update_conda_envs.rst(1 hunks)docs/workflow_developers/update_snakemake_wrappers.rst(1 hunks)docs/workflow_users/input_registration.rst(1 hunks)docs/workflow_users/workflow_deployment.rst(2 hunks)snakedeploy/__init__.py(0 hunks)snakedeploy/deploy.py(1 hunks)snakedeploy/providers.py(2 hunks)tests/test_client.sh(2 hunks)
💤 Files with no reviewable changes (2)
- snakedeploy/init.py
- docs/_static/theme.css
✅ Files skipped from review due to trivial changes (8)
- docs/workflow_developers/update_conda_envs.rst
- docs/snakemake_developers/scaffold_snakemake_plugins.rst
- docs/workflow_users/input_registration.rst
- docs/workflow_users/workflow_deployment.rst
- docs/getting_started/installation.rst
- snakedeploy/deploy.py
- docs/workflow_developers/update_snakemake_wrappers.rst
- docs/index.rst
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- snakedeploy/providers.py
🧰 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:
docs/conf.py
🧬 Code Graph Analysis (1)
tests/test_client.sh (2)
tests/helpers.sh (1)
runTest(1-26)snakedeploy/client.py (1)
main(245-317)
🪛 GitHub Actions: CI
docs/conf.py
[error] 1-1: Ruff formatting check failed. File would be reformatted. Run 'ruff format --write' to fix code style issues.
🪛 YAMLlint (1.37.1)
.readthedocs.yaml
[error] 16-16: no new line character at the end of file
(new-line-at-end-of-file)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.readthedocs.yaml(1 hunks)snakedeploy/providers.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .readthedocs.yaml
🧰 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:
snakedeploy/providers.py
🪛 Ruff (0.12.2)
snakedeploy/providers.py
58-58: Redefinition of unused clone from line 54
(F811)
66-66: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 GitHub Actions: CI
snakedeploy/providers.py
[error] 1-1: Ruff formatting check failed. File would be reformatted. Run 'ruff format --write' to fix code style issues.
🔇 Additional comments (2)
snakedeploy/providers.py (2)
2-3: LGTM: Import changes support the new functionality.The addition of
copytreeandshutilimports appropriately supports the updated file operation logic in theclonemethod.
1-67: Apply Ruff formatting fixes to snakedeploy/providers.pyA blank line is required before the second
def clonein theLocalclass to satisfy Ruff’s PEP8 checks. You can apply this automatically:--- snakedeploy/providers.py +++ snakedeploy/providers.py @@ -55,6 +55,7 @@ """ A local "clone" means moving files. """ + def clone(self, tmpdir: str): """ A local "clone" means moving files.Run:
ruff format --in-place snakedeploy/providers.pyThen re-run CI to verify the formatting errors are resolved.
…loy into feat/scaffold-plugins
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…loy into feat/scaffold-plugins
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
snakedeploy/scaffold_plugins/common.py (1)
68-71: Add error handling to the first subprocess call.The first
sp.runcall lacks error handling while subsequent calls usecheck=True. This inconsistency could cause silent failures when adding dependencies.Apply this diff to add error handling:
sp.run( ["pixi", "add", "--pypi", "snakemake-interface-common"] - + self.get_dependencies() + + self.get_dependencies(), + check=True, )snakedeploy/providers.py (1)
58-63: Improve exception chaining for better error traceability.The error handling implementation is good, but the exception should use proper chaining as indicated by static analysis.
Apply this diff to improve exception chaining:
except OSError as e: - raise UserError(f"Failed to remove existing directory {tmpdir}: {e}") + raise UserError(f"Failed to remove existing directory {tmpdir}: {e}") from eThis preserves the original traceback for better debugging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/conf.py(5 hunks)snakedeploy/deploy.py(1 hunks)snakedeploy/providers.py(2 hunks)snakedeploy/scaffold_plugins/common.py(1 hunks)snakedeploy/templates/plugins/software-deployment-plugins/init.py(1 hunks)snakedeploy/templates/plugins/storage-plugins/init.py(1 hunks)tests/test_client.sh(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- snakedeploy/deploy.py
🚧 Files skipped from review as they are similar to previous changes (1)
- snakedeploy/templates/plugins/software-deployment-plugins/init.py
🧰 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:
docs/conf.pysnakedeploy/scaffold_plugins/common.pysnakedeploy/templates/plugins/storage-plugins/init.pysnakedeploy/providers.py
🧬 Code Graph Analysis (2)
snakedeploy/scaffold_plugins/common.py (6)
snakedeploy/exceptions.py (1)
UserError(43-44)snakedeploy/scaffold_plugins/executor_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/report_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/scheduler_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/software_deployment_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/storage_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)
tests/test_client.sh (2)
tests/helpers.sh (1)
runTest(1-26)snakedeploy/client.py (1)
main(245-317)
🪛 Ruff (0.12.2)
snakedeploy/scaffold_plugins/common.py
35-35: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
37-37: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
snakedeploy/providers.py
62-62: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 Shellcheck (0.10.0)
tests/test_client.sh
[warning] 87-87: workdir appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 92-92: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: testing
🔇 Additional comments (10)
snakedeploy/scaffold_plugins/common.py (3)
1-28: LGTM! Well-structured abstract base class design.The imports are appropriate and the abstract base class follows good design patterns with clear separation between abstract methods that subclasses must implement and concrete methods that provide default behavior.
43-66: LGTM! Proper package validation and configuration.The logic correctly validates package naming conventions, extracts plugin names, and sets appropriate URLs and Python version requirements.
135-167: LGTM! Well-implemented template rendering logic.The Jinja2 template setup and rendering logic is well-structured with proper directory creation and template context passing. The solution for the hard-coded package name issue has been correctly implemented.
snakedeploy/templates/plugins/storage-plugins/init.py (3)
1-60: LGTM! Comprehensive storage plugin template with excellent documentation.The imports provide all necessary interfaces for storage plugin development, and the
StorageProviderSettingsclass offers detailed examples with well-documented metadata options that will help developers understand the available configuration features.
61-124: LGTM! Well-documented provider class template.The
StorageProviderclass provides comprehensive method stubs with excellent documentation explaining the purpose and expected implementation of each method. This will be very helpful for plugin developers.
125-241: LGTM! Comprehensive storage object template with proper patterns.The
StorageObjectclass demonstrates proper inheritance from multiple base classes, appropriate use of theretry_decoratorfor fallible operations, and comprehensive method coverage with excellent documentation. This provides a solid foundation for storage plugin development.snakedeploy/providers.py (1)
2-3: LGTM! Appropriate import changes for shutil usage.The addition of
shutilimports is consistent with the migration fromdistutils.dir_util.copy_treetoshutil.copytree.tests/test_client.sh (1)
63-63: LGTM! Appropriate change to use branch reference.The change from
--tag v1.0.0to--branch mainis appropriate for testing the deployment functionality with the current default branch.docs/conf.py (2)
16-42: LGTM! Well-configured theme migration.The migration to
sphinxawesome_themewith proper theme options and permalinks configuration is well-implemented. The removal of Markdown support and napoleon extension aligns with focusing on RST-only documentation.
99-110: LGTM! Comprehensive theme configuration with good ecosystem integration.The theme options are well-configured with appropriate logos, navigation links to the Snakemake ecosystem, and useful features like external link styling and header links enabled.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
snakedeploy/scaffold_plugins/common.py (1)
83-86: Add consistent error handling for subprocess calls.The first subprocess call lacks error checking while subsequent calls use
check=True. This inconsistency could lead to silent failures.Apply this diff to add error checking:
sp.run( ["pixi", "add", "--pypi", "snakemake-interface-common"] - + self.get_dependencies() + + self.get_dependencies(), + check=True )
🧹 Nitpick comments (1)
snakedeploy/scaffold_plugins/common.py (1)
157-164: Add error handling for template rendering operations.Template rendering operations could fail due to missing templates or file system issues. Consider adding error handling around the template rendering.
def render_template(name, dest: Path): - dest.parent.mkdir(exist_ok=True, parents=True) - with open(dest, "w") as f: - f.write( - templates.get_template("plugins/" + name).render( - pyproject=pyproject, plugin_name=plugin_name - ) - ) + try: + dest.parent.mkdir(exist_ok=True, parents=True) + with open(dest, "w") as f: + f.write( + templates.get_template("plugins/" + name).render( + pyproject=pyproject, plugin_name=plugin_name + ) + ) + except Exception as e: + raise UserError(f"Failed to render template {name}: {e}") from e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
snakedeploy/scaffold_plugins/common.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:
snakedeploy/scaffold_plugins/common.py
🧬 Code Graph Analysis (1)
snakedeploy/scaffold_plugins/common.py (6)
snakedeploy/exceptions.py (1)
UserError(43-44)snakedeploy/scaffold_plugins/executor_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/scheduler_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/report_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/software_deployment_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/storage_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)
🪛 Ruff (0.12.2)
snakedeploy/scaffold_plugins/common.py
45-46: SyntaxError: Expected an indented block after except clause
46-46: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
46-46: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-48: SyntaxError: Expected 'import', found newline
48-48: SyntaxError: Unexpected indentation
50-50: SyntaxError: Expected a statement
51-51: SyntaxError: Expected an indented block after function definition
51-51: SyntaxError: Expected an identifier, but found a keyword 'with' that cannot be used here
51-51: SyntaxError: Simple statements must be separated by newlines or semicolons
51-51: SyntaxError: Expected a statement
51-52: SyntaxError: Expected an expression
53-53: SyntaxError: Invalid annotated assignment target
53-53: SyntaxError: Expected an identifier, but found a keyword 'try' that cannot be used here
53-54: SyntaxError: Expected an expression
54-54: SyntaxError: Expected an identifier, but found a keyword 'with' that cannot be used here
54-54: SyntaxError: Simple statements must be separated by newlines or semicolons
54-54: SyntaxError: Expected a statement
54-55: SyntaxError: Expected an expression
56-56: SyntaxError: Expected an identifier, but found a keyword 'except' that cannot be used here
56-56: SyntaxError: Simple statements must be separated by newlines or semicolons
56-56: SyntaxError: Expected a statement
56-57: SyntaxError: Expected an expression
57-57: SyntaxError: Expected an identifier, but found a keyword 'raise' that cannot be used here
57-57: SyntaxError: Simple statements must be separated by newlines or semicolons
57-57: SyntaxError: Simple statements must be separated by newlines or semicolons
57-58: SyntaxError: Expected 'import', found newline
58-58: SyntaxError: Unexpected indentation
🪛 GitHub Actions: CI
snakedeploy/scaffold_plugins/common.py
[error] 45-45: Failed to parse file: Expected an indented block after except clause at line 45, column 27.
🔇 Additional comments (5)
snakedeploy/scaffold_plugins/common.py (5)
1-28: LGTM: Clean class structure and imports.The abstract base class design is well-structured, providing a clear contract for plugin-specific implementations while sharing common scaffolding logic.
29-37: LGTM: Proper error handling in nested function.The nested
load_pyprojectfunction correctly implements error handling with proper exception chaining as suggested in previous reviews.
58-81: Validate the main scaffolding logic after fixing syntax errors.The package validation and configuration logic appears sound, but ensure proper indentation within the
handlemethod once the syntax errors are resolved.
39-57: Fix critical syntax errors preventing code execution.The code contains severe syntax errors and malformed function definitions that prevent the file from being parsed. There are duplicate
load_pyprojectandsave_pyprojectfunctions with incorrect indentation and diff markers.Apply this diff to remove the malformed code and fix the syntax:
- -def load_pyproject(): - try: - with open("pyproject.toml", "r") as f: - pyproject = toml.load(f) - except FileNotFoundError: - raise UserError("pyproject.toml not found in current directory") - except Exception as e: -- raise UserError(f"Failed to read pyproject.toml: {e}") -+ raise UserError(f"Failed to read pyproject.toml: {e}") from e - return pyproject - -def save_pyproject(pyproject): -- with open("pyproject.toml", "w") as f: -- toml.dump(pyproject, f) -+ try: -+ with open("pyproject.toml", "w") as f: -+ toml.dump(pyproject, f) -+ except Exception as e: -+ raise UserError(f"Failed to write pyproject.toml: {e}") from e + def save_pyproject(pyproject): + try: + with open("pyproject.toml", "w") as f: + toml.dump(pyproject, f) + except Exception as e: + raise UserError(f"Failed to write pyproject.toml: {e}") from e +Likely an incorrect or invalid review comment.
115-130: Ignore test file path discrepancy.The plugin templates emit a file named tests.py in the tests/ directory (e.g. from tests.py.j2), which corresponds exactly to tests/tests.py used in the Pixi task. No change is needed.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/index.rst(2 hunks)docs/workflow_users/input_registration.rst(1 hunks)docs/workflow_users/workflow_deployment.rst(1 hunks)snakedeploy/scaffold_plugins/common.py(1 hunks)tests/test_client.sh(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/workflow_users/input_registration.rst
- docs/workflow_users/workflow_deployment.rst
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/index.rst
🧰 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:
snakedeploy/scaffold_plugins/common.py
🧬 Code Graph Analysis (1)
snakedeploy/scaffold_plugins/common.py (6)
snakedeploy/exceptions.py (1)
UserError(43-44)snakedeploy/scaffold_plugins/executor_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/scheduler_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/report_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/software_deployment_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)snakedeploy/scaffold_plugins/storage_plugins.py (3)
get_templates(8-14)get_plugin_type(16-17)include_snakemake_dev_dependency(19-20)
🪛 Ruff (0.12.2)
snakedeploy/scaffold_plugins/common.py
35-35: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 Shellcheck (0.10.0)
tests/test_client.sh
[warning] 92-92: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 95-95: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (1)
snakedeploy/scaffold_plugins/common.py (1)
1-171: Well-structured plugin scaffolding implementation!The abstract base class design with template rendering and comprehensive error handling provides a solid foundation for scaffolding different plugin types. The use of
check=Truefor subprocess calls and proper exception chaining ensures robust error handling.
🤖 I have created a release *beep* *boop* --- ## [0.12.0](v0.11.0...v0.12.0) (2025-07-25) ### Features * add plugin scaffolding ([#89](#89)) ([c1b8fa0](c1b8fa0)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary by CodeRabbit
New Features
Chores