feat: StorageProvider auto-selection on StorageClient API #6

Many of the code changes here are not complete repetitions, but are still very similar to one another. It might be more maintainable to define a function like:

def _execute_function_on_provider(self, path, funcname, *args, **kwargs):
    # Obtain a suitable provider for the path if it exists
    source_provider = self._helper_find_provider_if_needed(path)

    if source_provider != self.provider:
        try:
            return getattr(source_provider, funcname)(*args, **kwargs)
        except StorageError:
            # Always fall back to the current provider
        finally:
            if source_provider:
                source_provider.close()

    return getattr(self.provider, funcname)(*args, **kwargs)

Then you can call:

list(self._execute_func_on_provider('list_files', path, recursive)) in list_files
self._execute_func_on_provider('file_exists', path) in file_exists
self._execute_func_on_provider('get_file_size', path) in get_file_size

and so on.

Many of the code changes here are not complete repetitions, but are still very similar to one another. It might be more maintainable to define a function like: ``` def _execute_function_on_provider(self, path, funcname, *args, **kwargs): # Obtain a suitable provider for the path if it exists source_provider = self._helper_find_provider_if_needed(path) if source_provider != self.provider: try: return getattr(source_provider, funcname)(*args, **kwargs) except StorageError: # Always fall back to the current provider finally: if source_provider: source_provider.close() return getattr(self.provider, funcname)(*args, **kwargs) ``` Then you can call: - `list(self._execute_func_on_provider('list_files', path, recursive))` in `list_files` - `self._execute_func_on_provider('file_exists', path)` in `file_exists` - `self._execute_func_on_provider('get_file_size', path)` in `get_file_size` and so on.

src/clevercloud_storage_framework/client.py

					
				@ -38,0 +59,4 @@

				            else:

				                source_provider = StorageClientFactory.get_provider_for_path(path)

				        return self.provider if source_provider is None else source_provider

khird commented

2025-09-29 13:41:55 +00:00

Idiomatic Python might prefer return source_provider or self.provider

Idiomatic Python might prefer `return source_provider or self.provider`

CoreRasurae commented

2025-09-29 20:28:40 +00:00

I will take note of those suggestions for later, but i have some urgency on getting this merged, as CleverSwarm Endpoints S3 support depends on this.

Regarding the first suggestion yes, it makes the code more maintainable, however the code repetition is not that large and it may be easier to track in debug sessions. If a bug comes up in that particular section i will consider refactoring it your way.
As for the second, you are also right, i will probably open an issue with this suggestions.

I will take note of those suggestions for later, but i have some urgency on getting this merged, as CleverSwarm Endpoints S3 support depends on this. - Regarding the first suggestion yes, it makes the code more maintainable, however the code repetition is not that large and it may be easier to track in debug sessions. If a bug comes up in that particular section i will consider refactoring it your way. - As for the second, you are also right, i will probably open an issue with this suggestions.

CoreRasurae marked this conversation as resolved

CoreRasurae self-assigned this

2025-09-29 20:22:55 +00:00

CoreRasurae merged commit 7caa68abb6 into master

2025-09-29 20:30:13 +00:00

CoreRasurae deleted branch feat/storage_auto_selection-#5

2025-09-29 20:30:14 +00:00

CoreRasurae referenced this pull request

2025-09-29 20:37:39 +00:00

Improve code maintainability and readability in src/clevercloud_storage_framework/client.py #8

Sign in to join this conversation.

No reviewers