feat: on demand retrieval (via symlinking) in case input is annotated as on-demand eligible by Snakemake (e.g. because of sequential access being annotated by the developer #32
Conversation
… as on-demand eligible by Snakemake (e.g. because of sequential access being annotated by the developer
📝 WalkthroughWalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant S as StorageObject.retrieve_object
participant L as Logger
participant OS as os.symlink
participant R as sysrsync.get_rsync_command
participant C as _run_cmd
S->>S: Check is_ondemand_eligible
alt Eligible for on-demand retrieval
S->>L: Log "Creating symlink for on-demand retrieval"
S->>OS: Create symlink from query_path to local_path
else Not eligible for on-demand retrieval
S->>R: Construct rsync command
S->>C: Execute rsync command via _run_cmd
end
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
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:
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: 1
🧹 Nitpick comments (2)
snakemake_storage_plugin_fs/__init__.py (2)
206-210: Add error handling for symlink creationThe symlink creation doesn't have error handling for cases where it might fail (e.g., permission issues, crossing filesystem boundaries, or if the target already exists).
- os.symlink( - self.query_path, - self.local_path(), - target_is_directory=self.query_path.is_dir(), - ) + try: + os.symlink( + self.query_path, + self.local_path(), + target_is_directory=self.query_path.is_dir(), + ) + except OSError as e: + self.provider.logger.warning( + f"Failed to create symlink for on-demand retrieval of {self.query_path}. " + f"Falling back to rsync: {e}" + ) + cmd = sysrsync.get_rsync_command( + str(self.query_path), str(self.local_path()), options=["-av"] + ) + self._run_cmd(cmd)
203-205: Consider documenting symlink limitationsThe use of symlinks for on-demand retrieval is a good optimization, but has limitations that users should be aware of (e.g., symlinks don't work across different filesystems, break if the source is moved/deleted).
Consider adding a more detailed log message explaining these limitations:
- self.provider.logger.info( - f"Creating symlink for on-demand retrieval of {self.query_path}." - ) + self.provider.logger.info( + f"Creating symlink for on-demand retrieval of {self.query_path}. " + f"Note: Symlinks require source files to remain available and may not work across filesystems." + )
📜 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 (1)
snakemake_storage_plugin_fs/__init__.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.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 the `s...
**/*.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.
snakemake_storage_plugin_fs/__init__.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: formatting
🔇 Additional comments (1)
snakemake_storage_plugin_fs/__init__.py (1)
202-215: Good optimization for on-demand retrievalThe conditional logic to use symlinks for on-demand eligible objects is a good optimization that can improve performance, especially for large files that might not be accessed.
🤖 I have created a release *beep* *boop* --- ## [1.1.0](v1.0.6...v1.1.0) (2025-03-20) ### Features * on demand retrieval (via symlinking) in case input is annotated as on-demand eligible by Snakemake (e.g. because of sequential access being annotated by the developer ([#32](#32)) ([e69267c](e69267c)) ### Bug Fixes * adapt to interface version 4.0 ([fc6f499](fc6f499)) * proper use of relpath to define symlink ([2295f7f](2295f7f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit