Skip to content

Commit

Permalink
Fix an error when incomplete S3Client is destroyed
Browse files Browse the repository at this point in the history
If an error occurs during initialization of the S3Client instance, the
initializer for the Client superclass is never called. This could
happen, for example, when the S3Client has a bad profile name passed
which botocore doesn't understand. The specific error is that the
file_cache_mode attribute is not set because the superclass initializer
is not called in the S3Client initializer if an error occurs during S3
session setup. This commit moves the superclass initializer to the top
of the S3Client initializer to ensure that all superclass attributes are
defined if/when any errors occur.

The same fix is applied for the Google client and the local client.
  • Loading branch information
bryanwweber committed Oct 30, 2023
1 parent ffbdd1c commit d7261ca
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 17 deletions.
12 changes: 6 additions & 6 deletions cloudpathlib/gs/gsclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ def __init__(
content_type_method (Optional[Callable]): Function to call to guess media type (mimetype) when
writing a file to the cloud. Defaults to `mimetypes.guess_type`. Must return a tuple (content type, content encoding).
"""

super().__init__(
local_cache_dir=local_cache_dir,
content_type_method=content_type_method,
file_cache_mode=file_cache_mode,
)
if application_credentials is None:
application_credentials = os.getenv("GOOGLE_APPLICATION_CREDENTIALS")

Expand All @@ -92,12 +98,6 @@ def __init__(
except DefaultCredentialsError:
self.client = StorageClient.create_anonymous_client()

super().__init__(
local_cache_dir=local_cache_dir,
content_type_method=content_type_method,
file_cache_mode=file_cache_mode,
)

def _get_metadata(self, cloud_path: GSPath) -> Optional[Dict[str, Any]]:
bucket = self.client.bucket(cloud_path.bucket)
blob = bucket.get_blob(cloud_path.blob)
Expand Down
10 changes: 5 additions & 5 deletions cloudpathlib/local/localclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,17 @@ def __init__(
content_type_method: Optional[Callable] = mimetypes.guess_type,
**kwargs,
):
# setup caching and local versions of file. use default temp dir if not provided
if local_storage_dir is None:
local_storage_dir = self.get_default_storage_dir()
self._local_storage_dir = Path(local_storage_dir)

super().__init__(
local_cache_dir=local_cache_dir,
content_type_method=content_type_method,
file_cache_mode=file_cache_mode,
)

# setup caching and local versions of file. use default temp dir if not provided
if local_storage_dir is None:
local_storage_dir = self.get_default_storage_dir()
self._local_storage_dir = Path(local_storage_dir)

@classmethod
def get_default_storage_dir(cls) -> Path:
if cls._default_storage_temp_dir is None:
Expand Down
12 changes: 6 additions & 6 deletions cloudpathlib/s3/s3client.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ def __init__(
args that are supported look at the upload and download lists in the
[boto3 docs](https://boto3.amazonaws.com/v1/documentation/api/latest/reference/customizations/s3.html#boto3.s3.transfer.S3Transfer).
"""

super().__init__(
local_cache_dir=local_cache_dir,
content_type_method=content_type_method,
file_cache_mode=file_cache_mode,
)
endpoint_url = endpoint_url or os.getenv("AWS_ENDPOINT_URL")
if boto3_session is not None:
self.sess = boto3_session
Expand Down Expand Up @@ -127,12 +133,6 @@ def __init__(
if k in self._extra_args
}

super().__init__(
local_cache_dir=local_cache_dir,
content_type_method=content_type_method,
file_cache_mode=file_cache_mode,
)

def _get_metadata(self, cloud_path: S3Path) -> Dict[str, Any]:
# get accepts all download extra args
data = self.s3.ObjectSummary(cloud_path.bucket, cloud_path.key).get(
Expand Down

0 comments on commit d7261ca

Please sign in to comment.