fix: fix some lints #75
Conversation
📝 WalkthroughWalkthroughThe changes update type annotations and logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StorageProviderBase
participant Settings
participant StorageObjectRead
participant StorageObjectWrite
User->>StorageProviderBase: Instantiate (wait_for_free_local_storage: Optional[int])
StorageProviderBase->>Settings: Access max_requests_per_second
alt max_requests_per_second is set
StorageProviderBase->>StorageProviderBase: Use custom rate limit
else
StorageProviderBase->>StorageProviderBase: Use default rate limit
end
User->>StorageProviderBase: Access is_read_write property
StorageProviderBase->>StorageObjectRead: Check isinstance
StorageProviderBase->>StorageObjectWrite: Check isinstance
alt Both checks True
StorageProviderBase-->>User: Return True
else
StorageProviderBase-->>User: Return False
end
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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:
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: 2
🧹 Nitpick comments (1)
snakemake_interface_storage_plugins/storage_provider.py (1)
62-70: Docstring / help text missing for newOptional[int]parameterThe constructor now allows
wait_for_free_local_storage=None.
A short docstring or inline comment explaining the meaning ofNone(disable waiting) vs. a positive integer would make the API clearer for plugin authors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
snakemake_interface_storage_plugins/storage_provider.py(3 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_interface_storage_plugins/storage_provider.py
🧬 Code Graph Analysis (1)
snakemake_interface_storage_plugins/storage_provider.py (1)
snakemake_interface_storage_plugins/storage_object.py (2)
StorageObjectRead(123-258)StorageObjectWrite(261-284)
| StorageObjectRead, | ||
| StorageObjectWrite, | ||
| ) | ||
|
|
||
| return isinstance(self.storage_object_cls, StorageObjectReadWrite) | ||
| return isinstance( | ||
| self.get_storage_object_cls(), StorageObjectRead | ||
| ) and isinstance(self.get_storage_object_cls(), StorageObjectWrite) | ||
|
|
There was a problem hiding this comment.
is_read_write uses isinstance() on a class – should be issubclass()
self.get_storage_object_cls() returns a class, but isinstance() expects an instance.
As written, the expression will always evaluate to False, so is_read_write will never report True, even when the storage object supports both read and write.
- return isinstance(
- self.get_storage_object_cls(), StorageObjectRead
- ) and isinstance(self.get_storage_object_cls(), StorageObjectWrite)
+ cls = self.get_storage_object_cls()
+ return issubclass(cls, StorageObjectRead) and issubclass(cls, StorageObjectWrite)This also avoids the double call to get_storage_object_cls().
Failing to fix this will silently disable the read-write code paths that rely on this property.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| StorageObjectRead, | |
| StorageObjectWrite, | |
| ) | |
| return isinstance(self.storage_object_cls, StorageObjectReadWrite) | |
| return isinstance( | |
| self.get_storage_object_cls(), StorageObjectRead | |
| ) and isinstance(self.get_storage_object_cls(), StorageObjectWrite) | |
| StorageObjectRead, | |
| StorageObjectWrite, | |
| ) | |
| cls = self.get_storage_object_cls() | |
| return issubclass(cls, StorageObjectRead) and issubclass(cls, StorageObjectWrite) |
🤖 Prompt for AI Agents
In snakemake_interface_storage_plugins/storage_provider.py around lines 165 to
172, the is_read_write property incorrectly uses isinstance() on a class
returned by get_storage_object_cls(), which always returns False. Replace
isinstance() with issubclass() to correctly check if the class is a subclass of
StorageObjectRead and StorageObjectWrite. Also, call get_storage_object_cls()
once, store the result in a variable, and use that variable for both subclass
checks to avoid redundant calls.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit