Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvement: Add the ability to download archived files, and wait whi… #92

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

amiaynara
Copy link
Contributor

…le restore process is in progress

Task: https://basepairtech.atlassian.net/browse/WEB-15

basepair/api.py Outdated
message = f'File {key} is present in {storage_class}'
if restore_status == 'restore_error':
message = f'File with key {key} does not exist or you dont have permission'
elif storage_class == 'DEEP_ARCHIVE' and restore_status != 'restore_complete':
Copy link
Contributor

Choose a reason for hiding this comment

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

I would create a if block for restore_status != 'restore_complete' and put the storage class in there. further, there should be a default file_status_check_interval value in case the file doesn't have either of these 2 storage classes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Something like this -

if storage_class == 'a':
interval = x
expected_time = '10-12 hours'
elif storage_class == 'b':
interval = y
expected_time = '2-5 minutes'
else:
interval = z

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case the file doesn't have either of these 2 storage classes, we don't have to wait for file download. Because either the file is in STANDARD or does not exist, either way looks like there is no point in waiting, hence no need to set the interval. However, I have updated the code to have a default value, also refactored to reduce more lines of code.

@amiaynara amiaynara requested a review from ausinha August 10, 2022 08:17
@amiaynara amiaynara marked this pull request as draft August 10, 2022 08:56
@amiaynara amiaynara marked this pull request as ready for review August 11, 2022 04:07
@govind-basepair
Copy link
Contributor

Bump the version in basepair/init.py

@govind-basepair
Copy link
Contributor

There is a pylint warning -
basepair/api.py:1396:2: R0912: Too many branches (14/12) (too-many-branches)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants