Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,12 @@ The `cirro-cli configure` command creates a file in `PW_HOME` called `config.ini

You can set the `base_url` property in the config file rather than using the environment variable.

The `transfer_max_retries` configuration property specifies the maximum number of times to attempt uploading a file to Cirro in the event of a transfer failure.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add text:

After each failed upload attempt there will be a 1-2 minute pause before retrying the upload.

When uploading files to Cirro, network issues or temporary outages can occasionally cause a transfer to fail.
It will pause for an increasing amount of time for each retry attempt.

```ini
[General]
base_url = data-portal.io
transfer_max_retries = 15
```
9 changes: 7 additions & 2 deletions cirro/api/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,23 @@ class Constants:
home = os.environ.get('PW_HOME', '~/.cirro')
config_path = Path(home, 'config.ini').expanduser()
default_base_url = "data-portal.io"
default_max_retries = 10


class UserConfig(NamedTuple):
auth_method: str
auth_method_config: Dict # This needs to match the init params of the auth method
base_url: Optional[str]
transfer_max_retries: Optional[int]


def save_user_config(user_config: UserConfig):
original_user_config = load_user_config()
ini_config = configparser.SafeConfigParser()
ini_config['General'] = {
'auth_method': user_config.auth_method,
'base_url': Constants.default_base_url
'base_url': Constants.default_base_url,
'transfer_max_retries': Constants.default_max_retries
}
if original_user_config:
ini_config['General']['base_url'] = original_user_config.base_url
Expand All @@ -45,6 +48,7 @@ def load_user_config() -> Optional[UserConfig]:
main_config = ini_config['General']
auth_method = main_config.get('auth_method')
base_url = main_config.get('base_url')
transfer_max_retries = main_config.getint('transfer_max_retries', Constants.default_max_retries)

if auth_method and ini_config.has_section(auth_method):
auth_method_config = dict(ini_config[auth_method])
Expand All @@ -54,7 +58,8 @@ def load_user_config() -> Optional[UserConfig]:
return UserConfig(
auth_method=auth_method,
auth_method_config=auth_method_config,
base_url=base_url
base_url=base_url,
transfer_max_retries=transfer_max_retries
)
except Exception:
raise RuntimeError('Configuration load error, please re-run configuration')
Expand Down
3 changes: 2 additions & 1 deletion cirro/api/services/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ def upload_files(self, access_context: FileAccessContext, directory: str, files:
:return:
"""
s3_client = S3Client(partial(self.get_access_credentials, access_context), self._configuration.region)
upload_directory(directory, files, s3_client, access_context.bucket, access_context.path_prefix)
upload_directory(directory, files, s3_client, access_context.bucket, access_context.path_prefix,
max_retries=self._configuration.user_config.transfer_max_retries)

def download_files(self, access_context: FileAccessContext, directory: str, files: List[str]):
"""
Expand Down
10 changes: 7 additions & 3 deletions cirro/file_utils.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import os
import random
import time
from pathlib import Path, PurePath
from typing import List, Union

from boto3.exceptions import S3UploadFailedError
from botocore.exceptions import ConnectionError

from cirro.api.clients import S3Client
from cirro.api.models.file import DirectoryStatistics, File
Expand Down Expand Up @@ -86,11 +89,12 @@ def upload_directory(directory: str, files: List[str], s3_client: S3Client, buck
success = True

# Catch the upload error
except S3UploadFailedError as e:

except (S3UploadFailedError, ConnectionError) as e:
delay = random.uniform(0, 60) + retry * 60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So each retry takes longer and longer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it adds about a minute to each retry

# Report the error
print(f"Encountered error:\n{str(e)}\n"
f"Retrying ({max_retries - (retry + 1)} attempts remaining)")
f"Retrying in {delay:.0f} seconds ({max_retries - (retry + 1)} attempts remaining)")
time.sleep(delay)

if success:
break
Expand Down