From 508c8fc92e91e88fc0db74b0fbc445a96a05802d Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Fri, 13 Sep 2024 15:04:21 +0200 Subject: [PATCH 1/9] Standardized error messages --- cs3client/exceptions/exceptions.py | 13 +++++++------ cs3client/statuscodehandler.py | 20 ++++++-------------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/cs3client/exceptions/exceptions.py b/cs3client/exceptions/exceptions.py index 579cf27..cf8587e 100644 --- a/cs3client/exceptions/exceptions.py +++ b/cs3client/exceptions/exceptions.py @@ -2,6 +2,7 @@ exceptions.py Custom exception classes for the CS3 client. +Where applicable, the default values correspond to the Linux standard error strings. Authors: Rasmus Welander, Diogo Castro, Giuseppe Lo Presti. Emails: rasmus.oscar.welander@cern.ch, diogo.castro@cern.ch, giuseppe.lopresti@cern.ch @@ -14,7 +15,7 @@ class AuthenticationException(Exception): Standard error thrown when attempting an operation without the required access rights """ - def __init__(self, message: str = ""): + def __init__(self, message: str = "Operation not permitted"): super().__init__(message) @@ -23,7 +24,7 @@ class NotFoundException(IOError): Standard file missing message """ - def __init__(self, message: str = ""): + def __init__(self, message: str = "No such file or directory"): super().__init__(message) @@ -32,7 +33,7 @@ class SecretNotSetException(Exception): Standard file missing message """ - def __init__(self, message: str = ""): + def __init__(self, message: str = "Secret was not set, unable to authenticate"): super().__init__(message) @@ -42,7 +43,7 @@ class FileLockedException(IOError): or when a lock operation cannot be performed because of failed preconditions """ - def __init__(self, message: str = ""): + def __init__(self, message: str = "Lock mismatch"): super().__init__(message) @@ -60,7 +61,7 @@ class AlreadyExistsException(IOError): Standard error thrown when attempting to create a resource that already exists """ - def __init__(self, message: str = ""): + def __init__(self, message: str = "File exists"): super().__init__(message) @@ -69,5 +70,5 @@ class UnimplementedException(Exception): Standard error thrown when attempting to use a feature that is not implemented """ - def __init__(self, message: str = ""): + def __init__(self, message: str = "Not implemented"): super().__init__(message) diff --git a/cs3client/statuscodehandler.py b/cs3client/statuscodehandler.py index c468212..1c2d1fe 100644 --- a/cs3client/statuscodehandler.py +++ b/cs3client/statuscodehandler.py @@ -75,31 +75,23 @@ def handle_errors(self, status: cs3status.Status, operation: str, msg: str = Non if status.code == cs3code.CODE_FAILED_PRECONDITION or status.code == cs3code.CODE_ABORTED: self._log_precondition_info(status, operation, status_message, msg) - raise FileLockedException(f'Failed precondition: operation="{operation}" ' - f'status_code="{status.code}" message="{status.message}"') + raise FileLockedException if status.code == cs3code.CODE_ALREADY_EXISTS: self._log_already_exists(status, operation, status_message, msg) - raise AlreadyExistsException(f'Resource already exists: operation="{operation}" ' - f'status_code="{status.code}" message="{status.message}"') + raise AlreadyExistsException if status.code == cs3code.CODE_UNIMPLEMENTED: self._log.info(f'msg="Invoked {operation} on unimplemented feature" ') - raise UnimplementedException(f'Unimplemented feature: operation="{operation}" ' - f'status_code="{status.code}" message="{status.message}"') + raise UnimplementedException if status.code == cs3code.CODE_NOT_FOUND: self._log_not_found_info(status, operation, status_message, msg) - raise NotFoundException(f'Not found: operation="{operation}" ' - f'status_code="{status.code}" message="{status.message}"') + raise NotFoundException if status.code == cs3code.CODE_UNAUTHENTICATED: self._log_authentication_error(status, operation, status_message, msg) - raise AuthenticationException(f'Operation not permitted: operation="{operation}" ' - f'status_code="{status.code}" message="{status.message}"') + raise AuthenticationException if status.code != cs3code.CODE_OK: if "path not found" in str(status.message).lower(): self._log.info(f'msg="Invoked {operation} on missing file" ') - raise NotFoundException( - message=f'No such file or directory: operation="{operation}" ' - f'status_code="{status.code}" message="{status.message}"' - ) + raise NotFoundException self._log_unknown_error(status, operation, status_message, msg) raise UnknownException(f'Unknown Error: operation="{operation}" status_code="{status.code}" ' f'message="{status.message}"') From 5ba8f3413859e9b4e2fc7434df8279f769e7d039 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Fri, 13 Sep 2024 15:22:24 +0200 Subject: [PATCH 2/9] Fixed lock handling in write_file --- cs3client/file.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cs3client/file.py b/cs3client/file.py index ee08b82..dfe1142 100644 --- a/cs3client/file.py +++ b/cs3client/file.py @@ -170,8 +170,7 @@ def touch_file(self, auth_token: tuple, resource: Resource) -> None: self._log.debug(f'msg="Invoked TouchFile" trace="{res.status.trace}"') def write_file( - self, auth_token: tuple, resource: Resource, content: Union[str, bytes], size: int, - lock_md: tuple = ('', '') + self, auth_token: tuple, resource: Resource, content: Union[str, bytes], size: int, lock_md: tuple = None ) -> None: """ Write a file using the given userid as access token. The entire content is written @@ -183,14 +182,16 @@ def write_file( :param resource: Resource to write content to :param content: content to write :param size: size of content (optional) - :param lock_md: tuple (, ) + :param lock_md: tuple (, ) (optional) :return: None (Success) :raises: FileLockedException (File is locked), :raises: AuthenticationException (Authentication failed) :raises: UnknownException (Unknown error) """ - app_name, lock_id = lock_md + app_name = lock_id = '' + if lock_md: + app_name, lock_id = lock_md tstart = time.time() # prepare endpoint if size == -1: @@ -228,7 +229,7 @@ def write_file( "X-Reva-Transfer": protocol.token, **dict([auth_token]), "X-Lock-Id": lock_id, - "X-Lock_Holder": app_name, + "X-Lock-Holder": app_name, } putres = requests.put( url=protocol.upload_endpoint, From 63ed8bee29556b36bca981301d20825ec41fb6c1 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Fri, 13 Sep 2024 15:38:57 +0200 Subject: [PATCH 3/9] Fixed type in set_xattr --- cs3client/file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cs3client/file.py b/cs3client/file.py index dfe1142..e7d1b30 100644 --- a/cs3client/file.py +++ b/cs3client/file.py @@ -86,7 +86,7 @@ def set_xattr(self, auth_token: tuple, resource: Resource, key: str, value: str, :raises: UnknownException (Unknown error) """ md = cs3spr.ArbitraryMetadata() - md.metadata.update({key: value}) # pylint: disable=no-member + md.metadata.update({key: str(value)}) # pylint: disable=no-member req = cs3sp.SetArbitraryMetadataRequest(ref=resource.ref, arbitrary_metadata=md, lock_id=lock_id) res = self._gateway.SetArbitraryMetadata(request=req, metadata=[auth_token]) # CS3 storages may refuse to set an xattr in case of lock mismatch: this is an overprotection, From f8f393c418989fd80083ddb7474041eb4775b114 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Fri, 13 Sep 2024 16:01:07 +0200 Subject: [PATCH 4/9] Unnest the exceptions package --- cs3client/auth.py | 2 +- cs3client/exceptions/__init__.py | 74 ++++++++++++++++++++++++++++++ cs3client/exceptions/exceptions.py | 74 ------------------------------ cs3client/file.py | 2 +- cs3client/statuscodehandler.py | 2 +- docs/source/exceptions.rst | 2 +- tests/test_app.py | 2 +- tests/test_checkpoint.py | 2 +- tests/test_file.py | 2 +- tests/test_share.py | 2 +- tests/test_user.py | 2 +- 11 files changed, 83 insertions(+), 83 deletions(-) delete mode 100644 cs3client/exceptions/exceptions.py diff --git a/cs3client/auth.py b/cs3client/auth.py index a594094..7ac13c6 100644 --- a/cs3client/auth.py +++ b/cs3client/auth.py @@ -17,7 +17,7 @@ from cs3.rpc.v1beta1.code_pb2 import CODE_OK from .cs3client import CS3Client -from .exceptions.exceptions import AuthenticationException, SecretNotSetException +from .exceptions import AuthenticationException, SecretNotSetException from .config import Config diff --git a/cs3client/exceptions/__init__.py b/cs3client/exceptions/__init__.py index e69de29..0fe71a7 100644 --- a/cs3client/exceptions/__init__.py +++ b/cs3client/exceptions/__init__.py @@ -0,0 +1,74 @@ +""" +exceptions + +Custom exception classes for the CS3 client. +Where applicable, the default values correspond to the Linux standard error strings. + +Authors: Rasmus Welander, Diogo Castro, Giuseppe Lo Presti. +Emails: rasmus.oscar.welander@cern.ch, diogo.castro@cern.ch, giuseppe.lopresti@cern.ch +Last updated: 01/08/2024 +""" + + +class AuthenticationException(Exception): + """ + Standard error thrown when attempting an operation without the required access rights + """ + + def __init__(self, message: str = "Operation not permitted"): + super().__init__(message) + + +class NotFoundException(IOError): + """ + Standard file missing message + """ + + def __init__(self, message: str = "No such file or directory"): + super().__init__(message) + + +class SecretNotSetException(Exception): + """ + Standard file missing message + """ + + def __init__(self, message: str = "Secret was not set, unable to authenticate"): + super().__init__(message) + + +class FileLockedException(IOError): + """ + Standard error thrown when attempting to overwrite a file/xattr with a mistmatched lock, + or when a lock operation cannot be performed because of failed preconditions + """ + + def __init__(self, message: str = "Lock mismatch"): + super().__init__(message) + + +class UnknownException(Exception): + """ + Standard exception to be thrown when we get an error that is unknown, e.g. not defined in the cs3api + """ + + def __init__(self, message: str = ""): + super().__init__(message) + + +class AlreadyExistsException(IOError): + """ + Standard error thrown when attempting to create a resource that already exists + """ + + def __init__(self, message: str = "File exists"): + super().__init__(message) + + +class UnimplementedException(Exception): + """ + Standard error thrown when attempting to use a feature that is not implemented + """ + + def __init__(self, message: str = "Not implemented"): + super().__init__(message) diff --git a/cs3client/exceptions/exceptions.py b/cs3client/exceptions/exceptions.py deleted file mode 100644 index cf8587e..0000000 --- a/cs3client/exceptions/exceptions.py +++ /dev/null @@ -1,74 +0,0 @@ -""" -exceptions.py - -Custom exception classes for the CS3 client. -Where applicable, the default values correspond to the Linux standard error strings. - -Authors: Rasmus Welander, Diogo Castro, Giuseppe Lo Presti. -Emails: rasmus.oscar.welander@cern.ch, diogo.castro@cern.ch, giuseppe.lopresti@cern.ch -Last updated: 01/08/2024 -""" - - -class AuthenticationException(Exception): - """ - Standard error thrown when attempting an operation without the required access rights - """ - - def __init__(self, message: str = "Operation not permitted"): - super().__init__(message) - - -class NotFoundException(IOError): - """ - Standard file missing message - """ - - def __init__(self, message: str = "No such file or directory"): - super().__init__(message) - - -class SecretNotSetException(Exception): - """ - Standard file missing message - """ - - def __init__(self, message: str = "Secret was not set, unable to authenticate"): - super().__init__(message) - - -class FileLockedException(IOError): - """ - Standard error thrown when attempting to overwrite a file/xattr with a mistmatched lock, - or when a lock operation cannot be performed because of failed preconditions - """ - - def __init__(self, message: str = "Lock mismatch"): - super().__init__(message) - - -class UnknownException(Exception): - """ - Standard exception to be thrown when we get an error that is unknown, e.g. not defined in the cs3api - """ - - def __init__(self, message: str = ""): - super().__init__(message) - - -class AlreadyExistsException(IOError): - """ - Standard error thrown when attempting to create a resource that already exists - """ - - def __init__(self, message: str = "File exists"): - super().__init__(message) - - -class UnimplementedException(Exception): - """ - Standard error thrown when attempting to use a feature that is not implemented - """ - - def __init__(self, message: str = "Not implemented"): - super().__init__(message) diff --git a/cs3client/file.py b/cs3client/file.py index e7d1b30..efd9fee 100644 --- a/cs3client/file.py +++ b/cs3client/file.py @@ -20,7 +20,7 @@ from .config import Config -from .exceptions.exceptions import AuthenticationException, FileLockedException +from .exceptions import AuthenticationException, FileLockedException from .cs3resource import Resource from .statuscodehandler import StatusCodeHandler diff --git a/cs3client/statuscodehandler.py b/cs3client/statuscodehandler.py index 1c2d1fe..9205981 100644 --- a/cs3client/statuscodehandler.py +++ b/cs3client/statuscodehandler.py @@ -10,7 +10,7 @@ import cs3.rpc.v1beta1.code_pb2 as cs3code import cs3.rpc.v1beta1.status_pb2 as cs3status -from .exceptions.exceptions import AuthenticationException, NotFoundException, \ +from .exceptions import AuthenticationException, NotFoundException, \ UnknownException, AlreadyExistsException, FileLockedException, UnimplementedException from .config import Config diff --git a/docs/source/exceptions.rst b/docs/source/exceptions.rst index 5ba9cc0..3dd6db4 100644 --- a/docs/source/exceptions.rst +++ b/docs/source/exceptions.rst @@ -1,7 +1,7 @@ exceptions Module ================= -.. automodule:: exceptions.exceptions +.. automodule:: exceptions :members: :undoc-members: :show-inheritance: \ No newline at end of file diff --git a/tests/test_app.py b/tests/test_app.py index 0233797..11af905 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -12,7 +12,7 @@ from unittest.mock import Mock, patch import pytest -from cs3client.exceptions.exceptions import ( +from cs3client.exceptions import ( AuthenticationException, NotFoundException, UnknownException, diff --git a/tests/test_checkpoint.py b/tests/test_checkpoint.py index d005965..be6e63e 100644 --- a/tests/test_checkpoint.py +++ b/tests/test_checkpoint.py @@ -12,7 +12,7 @@ import pytest import cs3.rpc.v1beta1.code_pb2 as cs3code -from cs3client.exceptions.exceptions import ( +from cs3client.exceptions import ( AuthenticationException, NotFoundException, UnknownException, diff --git a/tests/test_file.py b/tests/test_file.py index 7e83afa..4120795 100644 --- a/tests/test_file.py +++ b/tests/test_file.py @@ -13,7 +13,7 @@ import cs3.rpc.v1beta1.code_pb2 as cs3code from cs3client.cs3resource import Resource -from cs3client.exceptions.exceptions import ( +from cs3client.exceptions import ( AuthenticationException, NotFoundException, FileLockedException, diff --git a/tests/test_share.py b/tests/test_share.py index 295c928..1185188 100644 --- a/tests/test_share.py +++ b/tests/test_share.py @@ -15,7 +15,7 @@ import cs3.storage.provider.v1beta1.resources_pb2 as cs3spr import cs3.rpc.v1beta1.code_pb2 as cs3code -from cs3client.exceptions.exceptions import ( +from cs3client.exceptions import ( AuthenticationException, NotFoundException, UnknownException, diff --git a/tests/test_user.py b/tests/test_user.py index 837b183..c62146a 100644 --- a/tests/test_user.py +++ b/tests/test_user.py @@ -12,7 +12,7 @@ from unittest.mock import Mock, patch import cs3.rpc.v1beta1.code_pb2 as cs3code -from cs3client.exceptions.exceptions import ( +from cs3client.exceptions import ( AuthenticationException, NotFoundException, UnknownException, From 2acec4eec206e15e5c2a55598386caf8ea377a72 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Mon, 23 Sep 2024 10:09:52 +0200 Subject: [PATCH 5/9] Better fix for lock metadata handling in write_file --- cs3client/file.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cs3client/file.py b/cs3client/file.py index efd9fee..7d84681 100644 --- a/cs3client/file.py +++ b/cs3client/file.py @@ -170,7 +170,8 @@ def touch_file(self, auth_token: tuple, resource: Resource) -> None: self._log.debug(f'msg="Invoked TouchFile" trace="{res.status.trace}"') def write_file( - self, auth_token: tuple, resource: Resource, content: Union[str, bytes], size: int, lock_md: tuple = None + self, auth_token: tuple, resource: Resource, content: Union[str, bytes], size: int, + app_name: str = None, lock_id: str = None ) -> None: """ Write a file using the given userid as access token. The entire content is written @@ -182,16 +183,14 @@ def write_file( :param resource: Resource to write content to :param content: content to write :param size: size of content (optional) - :param lock_md: tuple (, ) (optional) + :param app_name: application name (optional) + :param lock_id: lock id (optional) :return: None (Success) :raises: FileLockedException (File is locked), :raises: AuthenticationException (Authentication failed) :raises: UnknownException (Unknown error) """ - app_name = lock_id = '' - if lock_md: - app_name, lock_id = lock_md tstart = time.time() # prepare endpoint if size == -1: From 1985cdc70fb23996a7e8375455ec3bb140420913 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Tue, 1 Oct 2024 15:55:25 +0200 Subject: [PATCH 6/9] Fixed function signatures to uniformly use Optional[] as type hint where appropriate --- cs3client/app.py | 4 +- cs3client/checkpoint.py | 5 +- cs3client/file.py | 15 +++--- cs3client/share.py | 84 +++++++++++++++++----------------- cs3client/statuscodehandler.py | 16 ++++--- 5 files changed, 65 insertions(+), 59 deletions(-) diff --git a/cs3client/app.py b/cs3client/app.py index ea29c07..03b5cff 100644 --- a/cs3client/app.py +++ b/cs3client/app.py @@ -7,6 +7,8 @@ """ import logging +from typing import Optional + import cs3.app.registry.v1beta1.registry_api_pb2 as cs3arreg import cs3.app.registry.v1beta1.resources_pb2 as cs3arres import cs3.gateway.v1beta1.gateway_api_pb2 as cs3gw @@ -44,7 +46,7 @@ def __init__( self._config: Config = config def open_in_app( - self, auth_token: tuple, resource: Resource, view_mode: str = None, app: str = None + self, auth_token: tuple, resource: Resource, view_mode: Optional[str] = None, app: Optional[str] = None ) -> cs3apr.OpenInAppURL: """ Open a file in an app, given the resource, view mode (VIEW_MODE_VIEW_ONLY, VIEW_MODE_READ_ONLY, diff --git a/cs3client/checkpoint.py b/cs3client/checkpoint.py index dfedb00..b8c25d6 100644 --- a/cs3client/checkpoint.py +++ b/cs3client/checkpoint.py @@ -6,8 +6,9 @@ Last updated: 30/08/2024 """ -from typing import Generator +from typing import Generator, Optional import logging + import cs3.storage.provider.v1beta1.resources_pb2 as cs3spr import cs3.storage.provider.v1beta1.provider_api_pb2 as cs3spp from cs3.gateway.v1beta1.gateway_api_pb2_grpc import GatewayAPIStub @@ -65,7 +66,7 @@ def list_file_versions( return res.versions def restore_file_version( - self, auth_token: tuple, resource: Resource, version_key: str, lock_id: str = None + self, auth_token: tuple, resource: Resource, version_key: str, lock_id: Optional[str] = None ) -> None: """ Restore a file to a previous version. diff --git a/cs3client/file.py b/cs3client/file.py index 7d84681..3388010 100644 --- a/cs3client/file.py +++ b/cs3client/file.py @@ -10,8 +10,7 @@ import logging import http import requests -from typing import Union -from typing import Generator +from typing import Union, Optional, Generator import cs3.storage.provider.v1beta1.resources_pb2 as cs3spr import cs3.storage.provider.v1beta1.provider_api_pb2 as cs3sp from cs3.gateway.v1beta1.gateway_api_pb2_grpc import GatewayAPIStub @@ -71,7 +70,7 @@ def stat(self, auth_token: tuple, resource: Resource) -> cs3spr.ResourceInfo: ) return res.info - def set_xattr(self, auth_token: tuple, resource: Resource, key: str, value: str, lock_id: str = None) -> None: + def set_xattr(self, auth_token: tuple, resource: Resource, key: str, value: str, lock_id: Optional[str] = None) -> None: """ Set the extended attribute to for a resource. @@ -94,7 +93,7 @@ def set_xattr(self, auth_token: tuple, resource: Resource, key: str, value: str, self._status_code_handler.handle_errors(res.status, "set extended attribute", resource.get_file_ref_str()) self._log.debug(f'msg="Invoked setxattr" trace="{res.status.trace}"') - def remove_xattr(self, auth_token: tuple, resource: Resource, key: str, lock_id: str = None) -> None: + def remove_xattr(self, auth_token: tuple, resource: Resource, key: str, lock_id: Optional[str] = None) -> None: """ Remove the extended attribute . @@ -113,7 +112,7 @@ def remove_xattr(self, auth_token: tuple, resource: Resource, key: str, lock_id: self._log.debug(f'msg="Invoked UnsetArbitraryMetaData" trace="{res.status.trace}"') def rename_file( - self, auth_token: tuple, resource: Resource, newresource: Resource, lock_id: str = None + self, auth_token: tuple, resource: Resource, newresource: Resource, lock_id: Optional[str] = None ) -> None: """ Rename/move resource to new resource. @@ -133,7 +132,7 @@ def rename_file( self._status_code_handler.handle_errors(res.status, "rename file", resource.get_file_ref_str()) self._log.debug(f'msg="Invoked Move" trace="{res.status.trace}"') - def remove_file(self, auth_token: tuple, resource: Resource, lock_id: str = None) -> None: + def remove_file(self, auth_token: tuple, resource: Resource, lock_id: Optional[str] = None) -> None: """ Remove a resource. @@ -171,7 +170,7 @@ def touch_file(self, auth_token: tuple, resource: Resource) -> None: def write_file( self, auth_token: tuple, resource: Resource, content: Union[str, bytes], size: int, - app_name: str = None, lock_id: str = None + app_name: Optional[str] = None, lock_id: Optional[str] = None ) -> None: """ Write a file using the given userid as access token. The entire content is written @@ -270,7 +269,7 @@ def write_file( f'elapsedTimems="{(tend - tstart) * 1000:.1f}"' ) - def read_file(self, auth_token: tuple, resource: Resource, lock_id: str = None) -> Generator[bytes, None, None]: + def read_file(self, auth_token: tuple, resource: Resource, lock_id: Optional[str] = None) -> Generator[bytes, None, None]: """ Read a file. Note that the function is a generator, managed by the app server. diff --git a/cs3client/share.py b/cs3client/share.py index da401a3..0e180ad 100644 --- a/cs3client/share.py +++ b/cs3client/share.py @@ -7,6 +7,8 @@ """ import logging +from typing import Optional + import cs3.sharing.collaboration.v1beta1.collaboration_api_pb2 as cs3scapi from cs3.gateway.v1beta1.gateway_api_pb2_grpc import GatewayAPIStub import cs3.sharing.collaboration.v1beta1.resources_pb2 as cs3scr @@ -87,7 +89,7 @@ def create_share( return res.share def list_existing_shares( - self, auth_token: tuple, filter_list: list[cs3scr.Filter] = None, page_size: int = 0, page_token: str = None + self, auth_token: tuple, filter_list: list[cs3scr.Filter] = None, page_size: int = 0, page_token: Optional[str] = None ) -> list[cs3scr.Share]: """ List shares based on a filter. @@ -109,7 +111,7 @@ def list_existing_shares( ) return (res.share_infos, res.next_page_token) - def get_share(self, auth_token: tuple, opaque_id: str = None, share_key: cs3scr.ShareKey = None) -> cs3scr.Share: + def get_share(self, auth_token: tuple, opaque_id: Optional[str] = None, share_key: Optional[cs3scr.ShareKey] = None) -> cs3scr.Share: """ Get a share by its opaque id or share key (combination of resource_id, grantee and owner), one of them is required. @@ -141,7 +143,7 @@ def get_share(self, auth_token: tuple, opaque_id: str = None, share_key: cs3scr. ) return res.share - def remove_share(self, auth_token: tuple, opaque_id: str = None, share_key: cs3scr.ShareKey = None) -> None: + def remove_share(self, auth_token: tuple, opaque_id: Optional[str] = None, share_key: Optional[cs3scr.ShareKey] = None) -> None: """ Remove a share by its opaque id or share key (combination of resource_id, grantee and owner), one of them is required. @@ -175,9 +177,9 @@ def remove_share(self, auth_token: tuple, opaque_id: str = None, share_key: cs3s def update_share( self, auth_token: tuple, role: str, - opaque_id: str = None, - share_key: cs3scr.ShareKey = None, - display_name: str = None + opaque_id: Optional[str] = None, + share_key: Optional[cs3scr.ShareKey] = None, + display_name: Optional[str] = None ) -> cs3scr.Share: """ Update a share by its opaque id. @@ -216,7 +218,7 @@ def update_share( return res.share def list_received_existing_shares( - self, auth_token: tuple, filter_list: list = None, page_size: int = 0, page_token: str = None + self, auth_token: tuple, filter_list: Optional[list] = None, page_size: int = 0, page_token: Optional[str] = None ) -> list: """ List received existing shares. @@ -240,7 +242,7 @@ def list_received_existing_shares( return (res.share_infos, res.next_page_token) def get_received_share( - self, auth_token: tuple, opaque_id: str = None, share_key: cs3scr.ShareKey = None + self, auth_token: tuple, opaque_id: Optional[str] = None, share_key: Optional[cs3scr.ShareKey] = None ) -> cs3scr.ReceivedShare: """ Get a received share by its opaque id or share key (combination of resource_id, grantee and owner), @@ -313,12 +315,12 @@ def create_public_share( auth_token: tuple, resource_info: cs3spr.ResourceInfo, role: str, - password: str = None, - expiration: cs3types.Timestamp = None, - description: str = None, + password: Optional[str] = None, + expiration: Optional[cs3types.Timestamp] = None, + description: Optional[str] = None, internal: bool = False, notify_uploads: bool = False, - notify_uploads_extra_recipients: list = None, + notify_uploads_extra_recipients: Optional[list] = None, ) -> cs3slr.PublicShare: """ Create a public share. @@ -358,7 +360,7 @@ def create_public_share( return res.share def list_existing_public_shares( - self, auth_token: tuple, filter_list: list = None, page_size: int = 0, page_token: str = None, sign: bool = None + self, auth_token: tuple, filter_list: Optional[list] = None, page_size: int = 0, page_token: Optional[str] = None, sign: bool = False ) -> list: """ List existing public shares. @@ -385,7 +387,7 @@ def list_existing_public_shares( return (res.share_infos, res.next_page_token) def get_public_share( - self, auth_token: tuple, opaque_id: str = None, token: str = None, sign: bool = False + self, auth_token: tuple, opaque_id: Optional[str] = None, token: Optional[str] = None, sign: bool = False ) -> cs3slr.PublicShare: """ Get a public share by its opaque id or token, one of them is required. @@ -423,14 +425,14 @@ def update_public_share( auth_token: tuple, type: str, role: str, - opaque_id: str = None, - token: str = None, - password: str = None, - expiration: cs3types.Timestamp = None, - notify_uploads_extra_recipients: str = None, - description: str = None, - display_name: str = None, - notify_uploads: bool = None, + opaque_id: Optional[str] = None, + token: Optional[str] = None, + password: Optional[str] = None, + expiration: Optional[cs3types.Timestamp] = None, + notify_uploads_extra_recipients: Optional[str] = None, + description: Optional[str] = None, + display_name: Optional[str] = None, + notify_uploads: bool = False, ) -> None: """ Update a public share by its opaque id or token. (one of them is required), role and type are required, @@ -486,7 +488,7 @@ def update_public_share( ) return res.share - def remove_public_share(self, auth_token: tuple, token: str = None, opaque_id: str = None) -> None: + def remove_public_share(self, auth_token: tuple, token: Optional[str] = None, opaque_id: Optional[str] = None) -> None: """ Remove a public share by its token or opaque id, one of them is required. @@ -520,12 +522,12 @@ def _create_public_share_update( cls, type: str, role: str, - password: str = None, - expiration: cs3types.Timestamp = None, - display_name: str = None, - description: str = None, - notify_uploads: bool = None, - notify_uploads_extra_recipients: str = None, + password: Optional[str] = None, + expiration: Optional[cs3types.Timestamp] = None, + display_name: Optional[str] = None, + description: Optional[str] = None, + notify_uploads: bool = False, + notify_uploads_extra_recipients: Optional[str] = None, ) -> cs3slr.PublicShare: """ Create a public share update object, based on the type the property will be updated, @@ -567,7 +569,7 @@ def _create_public_share_update( @classmethod def _create_share_grant( - cls, opaque_id: str, idp: str, role: str, grantee_type: str, expiration: cs3types.Timestamp = None + cls, opaque_id: str, idp: str, role: str, grantee_type: str, expiration: Optional[cs3types.Timestamp] = None ) -> cs3scr.ShareGrant: """ Create a share grant object. @@ -643,10 +645,10 @@ def create_public_share_filter( cls, filter_type: str, resource_id: cs3spr.ResourceId = None, - owner_idp: str = None, - owner_opaque_id: str = None, - creator_idp: str = None, - creator_opaque_id: str = None, + owner_idp: Optional[str] = None, + owner_opaque_id: Optional[str] = None, + creator_idp: Optional[str] = None, + creator_opaque_id: Optional[str] = None, ) -> cs3slapi.ListPublicSharesRequest.Filter: """ Create a public share filter object, based on the filter type (can be TYPE_RESOURCE_ID, TYPE_OWNER, @@ -688,13 +690,13 @@ def create_share_filter( cls, filter_type: str, resource_id: cs3spr.ResourceId = None, - owner_idp: str = None, - owner_opaque_id: str = None, - creator_idp: str = None, - creator_opaque_id: str = None, - grantee_type: str = None, - space_id: str = None, - share_state: str = None, + owner_idp: Optional[str] = None, + owner_opaque_id: Optional[str] = None, + creator_idp: Optional[str] = None, + creator_opaque_id: Optional[str] = None, + grantee_type: Optional[str] = None, + space_id: Optional[str] = None, + share_state: Optional[str] = None, ) -> cs3scr.Filter: """ Create a share filter object, based on the filter type (can be TYPE_RESOURCE_ID, TYPE_OWNER, TYPE_CREATOR, diff --git a/cs3client/statuscodehandler.py b/cs3client/statuscodehandler.py index 9205981..0682388 100644 --- a/cs3client/statuscodehandler.py +++ b/cs3client/statuscodehandler.py @@ -7,6 +7,8 @@ """ import logging +from typing import Optional + import cs3.rpc.v1beta1.code_pb2 as cs3code import cs3.rpc.v1beta1.status_pb2 as cs3status @@ -20,7 +22,7 @@ def __init__(self, log: logging.Logger, config: Config) -> None: self._log = log self._config = config - def _log_not_found_info(self, status: cs3status.Status, operation: str, status_msg: str, msg: str = None) -> None: + def _log_not_found_info(self, status: cs3status.Status, operation: str, status_msg: str, msg: Optional[str] = None) -> None: self._log.info( f'msg="Not found on {operation}" {msg + " " if msg else ""} ' f'userid="{self._config.auth_client_id if self._config.auth_client_id else "no_id_set"}" ' @@ -28,7 +30,7 @@ def _log_not_found_info(self, status: cs3status.Status, operation: str, status_m ) def _log_authentication_error( - self, status: cs3status.Status, operation: str, status_msg: str, msg: str = None + self, status: cs3status.Status, operation: str, status_msg: str, msg: Optional[str] = None ) -> None: self._log.error( f'msg="Authentication failed on {operation}" {msg + " " if msg else ""}' @@ -36,7 +38,7 @@ def _log_authentication_error( f'trace="{status.trace}" reason="{status_msg}"' ) - def _log_unknown_error(self, status: cs3status.Status, operation: str, status_msg: str, msg: str = None) -> None: + def _log_unknown_error(self, status: cs3status.Status, operation: str, status_msg: str, msg: Optional[str] = None) -> None: self._log.error( f'msg="Failed to {operation}, unknown error" {msg + " " if msg else ""}' f'userid="{self._config.auth_client_id if self._config.auth_client_id else "no_id_set"}" ' @@ -44,7 +46,7 @@ def _log_unknown_error(self, status: cs3status.Status, operation: str, status_ms ) def _log_precondition_info( - self, status: cs3status.Status, operation: str, status_msg: str, msg: str = None + self, status: cs3status.Status, operation: str, status_msg: str, msg: Optional[str] = None ) -> None: self._log.info( f'msg="Failed precondition on {operation}" {msg + " " if msg else ""}' @@ -52,21 +54,21 @@ def _log_precondition_info( f'trace="{status.trace}" reason="{status_msg}"' ) - def _log_already_exists(self, status: cs3status.Status, operation: str, status_msg: str, msg: str = None) -> None: + def _log_already_exists(self, status: cs3status.Status, operation: str, status_msg: str, msg: Optional[str] = None) -> None: self._log.info( f'msg="Already exists on {operation}" {msg + " " if msg else ""}' f'userid="{self._config.auth_client_id if self._config.auth_client_id else "no_id_set"}" ' f'trace="{status.trace}" reason="{status_msg}"' ) - def _log_unimplemented(self, status: cs3status.Status, operation: str, status_msg: str, msg: str = None) -> None: + def _log_unimplemented(self, status: cs3status.Status, operation: str, status_msg: str, msg: Optional[str] = None) -> None: self._log.info( f'msg="Invoked {operation} on unimplemented feature" {msg + " " if msg else ""}' f'userid="{self._config.auth_client_id if self._config.auth_client_id else "no_id_set"}" ' f'trace="{status.trace}" reason="{status_msg}"' ) - def handle_errors(self, status: cs3status.Status, operation: str, msg: str = None) -> None: + def handle_errors(self, status: cs3status.Status, operation: str, msg: Optional[str] = None) -> None: if status.code == cs3code.CODE_OK: return From 897250d50dd084173873496634dd91aeb3e10cbd Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Tue, 1 Oct 2024 16:27:36 +0200 Subject: [PATCH 7/9] Do not force a type conversion as the type is hinted in the signature --- cs3client/file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cs3client/file.py b/cs3client/file.py index 3388010..e8a5554 100644 --- a/cs3client/file.py +++ b/cs3client/file.py @@ -85,7 +85,7 @@ def set_xattr(self, auth_token: tuple, resource: Resource, key: str, value: str, :raises: UnknownException (Unknown error) """ md = cs3spr.ArbitraryMetadata() - md.metadata.update({key: str(value)}) # pylint: disable=no-member + md.metadata.update({key: value}) # pylint: disable=no-member req = cs3sp.SetArbitraryMetadataRequest(ref=resource.ref, arbitrary_metadata=md, lock_id=lock_id) res = self._gateway.SetArbitraryMetadata(request=req, metadata=[auth_token]) # CS3 storages may refuse to set an xattr in case of lock mismatch: this is an overprotection, From 6d463631402e5d99760116e839a0ee7ce1f2f518 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Thu, 3 Oct 2024 10:42:40 +0200 Subject: [PATCH 8/9] Introduced a permission denied exception --- cs3client/exceptions/__init__.py | 28 +++++++++++++++++++--------- cs3client/statuscodehandler.py | 14 +++++++++++++- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/cs3client/exceptions/__init__.py b/cs3client/exceptions/__init__.py index 0fe71a7..8d38fcd 100644 --- a/cs3client/exceptions/__init__.py +++ b/cs3client/exceptions/__init__.py @@ -19,6 +19,15 @@ def __init__(self, message: str = "Operation not permitted"): super().__init__(message) +class PermissionDeniedException(IOError): + """ + Standard permission denied message + """ + + def __init__(self, message: str = "Permission denied"): + super().__init__(message) + + class NotFoundException(IOError): """ Standard file missing message @@ -47,15 +56,6 @@ def __init__(self, message: str = "Lock mismatch"): super().__init__(message) -class UnknownException(Exception): - """ - Standard exception to be thrown when we get an error that is unknown, e.g. not defined in the cs3api - """ - - def __init__(self, message: str = ""): - super().__init__(message) - - class AlreadyExistsException(IOError): """ Standard error thrown when attempting to create a resource that already exists @@ -72,3 +72,13 @@ class UnimplementedException(Exception): def __init__(self, message: str = "Not implemented"): super().__init__(message) + + +class UnknownException(Exception): + """ + Standard exception to be thrown when we get an error that is unknown, e.g. not defined in the cs3api + """ + + def __init__(self, message: str = ""): + super().__init__(message) + diff --git a/cs3client/statuscodehandler.py b/cs3client/statuscodehandler.py index 0682388..f10fc10 100644 --- a/cs3client/statuscodehandler.py +++ b/cs3client/statuscodehandler.py @@ -12,7 +12,7 @@ import cs3.rpc.v1beta1.code_pb2 as cs3code import cs3.rpc.v1beta1.status_pb2 as cs3status -from .exceptions import AuthenticationException, NotFoundException, \ +from .exceptions import AuthenticationException, PermissionDeniedException, NotFoundException, \ UnknownException, AlreadyExistsException, FileLockedException, UnimplementedException from .config import Config @@ -38,6 +38,15 @@ def _log_authentication_error( f'trace="{status.trace}" reason="{status_msg}"' ) + def _log_permission_denied_info( + self, status: cs3status.Status, operation: str, status_msg: str, msg: Optional[str] = None + ) -> None: + self._log.info( + f'msg="Permission denied on {operation}" {msg + " " if msg else ""}' + f'userid="{self._config.auth_client_id if self._config.auth_client_id else "no_id_set"}" ' + f'trace="{status.trace}" reason="{status_msg}"' + ) + def _log_unknown_error(self, status: cs3status.Status, operation: str, status_msg: str, msg: Optional[str] = None) -> None: self._log.error( f'msg="Failed to {operation}, unknown error" {msg + " " if msg else ""}' @@ -90,6 +99,9 @@ def handle_errors(self, status: cs3status.Status, operation: str, msg: Optional[ if status.code == cs3code.CODE_UNAUTHENTICATED: self._log_authentication_error(status, operation, status_message, msg) raise AuthenticationException + if status.code == cs3code.CODE_PERMISSION_DENIED: + self._log_permission_denied_info(status, operation, status_message, msg) + raise PermissionDeniedException if status.code != cs3code.CODE_OK: if "path not found" in str(status.message).lower(): self._log.info(f'msg="Invoked {operation} on missing file" ') From ae092cb59d7a71a2e517eecf929bb1fc7469e126 Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Tue, 22 Oct 2024 14:19:35 +0200 Subject: [PATCH 9/9] One more optional parameter --- cs3client/file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cs3client/file.py b/cs3client/file.py index 239bed6..092f260 100644 --- a/cs3client/file.py +++ b/cs3client/file.py @@ -171,7 +171,7 @@ def touch_file(self, auth_token: tuple, resource: Resource) -> None: def write_file( self, auth_token: tuple, resource: Resource, content: Union[str, bytes], size: int, app_name: Optional[str] = None, lock_id: Optional[str] = None, - disable_versioning: bool = False + disable_versioning: Optional[bool] = False ) -> None: """ Write a file using the given userid as access token. The entire content is written