Skip to content

U
uppy-s3_multipart
Unverified Commit e1760e54 by Janko Marohnić
Options

Don't retry aborting multipart upload

Even though the AWS S3 documentation states that we should do it, neither Uppy Companion nor tusd are doing this and nobody complained, so apparently there is no need. If it was really that important, I would expect aws-sdk-s3 to have a method for "safe aborting". Since it's good to take every opportunity to make code simpler, we remove the code that retries aborting the multipart upload.
parent 4fa91c48
Showing with 2 additions and 35 deletions
lib/uppy/s3_multipart/client.rb
...@@ -56,13 +56,7 @@ module Uppy ...@@ -56,13 +56,7 @@ module Uppy
def abort_multipart_upload(upload_id:, key:, **options) def abort_multipart_upload(upload_id:, key:, **options)
multipart_upload = multipart_upload(upload_id, key) multipart_upload = multipart_upload(upload_id, key)
multipart_upload.abort(**options)
# aws-sdk-s3 docs recommend retrying the abort in case the multipart
# upload still has parts
loop do
multipart_upload.abort(**options)
break unless multipart_upload.parts.any?
end
{} {}
end end
......
test/client_test.rb
...@@ -151,39 +151,12 @@ describe Uppy::S3Multipart::Client do ...@@ -151,39 +151,12 @@ describe Uppy::S3Multipart::Client do
it "aborts the multipart upload" do it "aborts the multipart upload" do
@client.abort_multipart_upload(upload_id: "bar", key: "foo") @client.abort_multipart_upload(upload_id: "bar", key: "foo")
assert_equal :abort_multipart_upload, @s3.api_requests[0][:operation_name] assert_equal 1, @s3.api_requests.count
assert_equal "foo", @s3.api_requests[0][:params][:key]
assert_equal "bar", @s3.api_requests[0][:params][:upload_id]
assert_equal "my-bucket", @s3.api_requests[0][:params][:bucket]
end
it "retries the abort if it failed" do
@s3.stub_responses(:list_parts, [
{ parts: [{ part_number: 1, etag: "etag" }] }, # first call
{ parts: [] }, # second call
])
@client.abort_multipart_upload(upload_id: "bar", key: "foo")
assert_equal :abort_multipart_upload, @s3.api_requests[0][:operation_name] assert_equal :abort_multipart_upload, @s3.api_requests[0][:operation_name]
assert_equal "foo", @s3.api_requests[0][:params][:key] assert_equal "foo", @s3.api_requests[0][:params][:key]
assert_equal "bar", @s3.api_requests[0][:params][:upload_id] assert_equal "bar", @s3.api_requests[0][:params][:upload_id]
assert_equal "my-bucket", @s3.api_requests[0][:params][:bucket] assert_equal "my-bucket", @s3.api_requests[0][:params][:bucket]
assert_equal :list_parts, @s3.api_requests[1][:operation_name]
assert_equal "foo", @s3.api_requests[1][:params][:key]
assert_equal "bar", @s3.api_requests[1][:params][:upload_id]
assert_equal "my-bucket", @s3.api_requests[1][:params][:bucket]
assert_equal :abort_multipart_upload, @s3.api_requests[2][:operation_name]
assert_equal "foo", @s3.api_requests[2][:params][:key]
assert_equal "bar", @s3.api_requests[2][:params][:upload_id]
assert_equal "my-bucket", @s3.api_requests[2][:params][:bucket]
assert_equal :list_parts, @s3.api_requests[3][:operation_name]
assert_equal "foo", @s3.api_requests[3][:params][:key]
assert_equal "bar", @s3.api_requests[3][:params][:upload_id]
assert_equal "my-bucket", @s3.api_requests[3][:params][:bucket]
end end
it "returns empty result" do it "returns empty result" do
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment