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
Cleanup incomplete upload upon error #599
Conversation
minio/api.py
Outdated
@@ -1524,7 +1524,8 @@ def _stream_put_object(self, bucket_name, object_name, | |||
|
|||
part_data = read_full(data, current_part_size) | |||
# Append current part information | |||
parts_to_upload.append((bucket_name, object_name, upload_id, part_number, part_data)) | |||
parts_to_upload.append((bucket_name, object_name, upload_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just a formatting change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
minio/api.py
Outdated
@@ -1524,7 +1524,8 @@ def _stream_put_object(self, bucket_name, object_name, | |||
|
|||
part_data = read_full(data, current_part_size) | |||
# Append current part information | |||
parts_to_upload.append((bucket_name, object_name, upload_id, part_number, part_data)) | |||
parts_to_upload.append((bucket_name, object_name, upload_id, | |||
part_number, part_data)) | |||
|
|||
# Run parts upload in parallel | |||
pool.parallel_run(self._upload_part_routine, parts_to_upload) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pool.parallel_run() can raise an exception, I think it would be good to catch it, remove incomplete upload and throw the exception again.
try:
pool.parallel_run(self._upload_part_routine, parts_to_upload)
except:
self._remove_incomplete_upload(bucket_name,
object_name,
upload_id)
raise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
67e2c50
to
1255abe
Compare
minio/api.py
Outdated
self._remove_incomplete_upload(bucket_name, | ||
object_name, | ||
upload_id) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A missing raise here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Since we do not have resuming support, we should simply cleanup failed transfers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
upload_id, | ||
uploaded_parts, | ||
metadata=metadata) | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there is formatting issue here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't see any issue, if there is a formatting issue python won't really compile.
Since we do not have resuming support, we should
simply cleanup failed transfers.