From 05ec58da53cb31a41def122feb69d0ad0b8da813 Mon Sep 17 00:00:00 2001 From: Javier Lopez Date: Wed, 2 Jul 2014 10:49:41 +0200 Subject: [PATCH 1/2] Fixes issue #834 Rewrite wait_for_upload_id function so UploadCancelledError is raised when state is _CANCELLED, previous logic didn't raise UploadCancelledError if upload_id was set. --- awscli/customizations/s3/tasks.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/awscli/customizations/s3/tasks.py b/awscli/customizations/s3/tasks.py index d187fb105530..430004ca3ae8 100644 --- a/awscli/customizations/s3/tasks.py +++ b/awscli/customizations/s3/tasks.py @@ -560,10 +560,10 @@ def wait_for_parts_to_finish(self): def wait_for_upload_id(self): with self._upload_id_condition: - while self._upload_id is None: - if self._state == self._CANCELLED: - raise UploadCancelledError("Upload has been cancelled.") - self._upload_id_condition.wait(timeout=1) + while self._upload_id is None and self._state != self._CANCELLED: + self._upload_id_condition.wait(timeout=1) + if self._state == self._CANCELLED: + raise UploadCancelledError("Upload has been cancelled.") return self._upload_id def wait_for_completion(self): From a65b23108c503106771be45efc59227ab35b9346 Mon Sep 17 00:00:00 2001 From: Javier Lopez Date: Fri, 4 Jul 2014 16:27:35 +0200 Subject: [PATCH 2/2] Added test_cancel_after_upload_id to test_tasks This test creates an upload_task, sets the upload_id, and then cancels the upload. At this point any call to wait_for_upload_id must raise an UploadCancelledError, even if a valid upload_id is set on the MultipartUploadContext object. --- tests/unit/customizations/s3/test_tasks.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/unit/customizations/s3/test_tasks.py b/tests/unit/customizations/s3/test_tasks.py index 051d21035076..4451c85cb569 100644 --- a/tests/unit/customizations/s3/test_tasks.py +++ b/tests/unit/customizations/s3/test_tasks.py @@ -223,6 +223,26 @@ def test_can_cancel_tasks(self): with self.assertRaises(UploadCancelledError): self.context.wait_for_parts_to_finish() + def test_cancel_after_upload_id(self): + # We want have a thread waiting for the upload id. + upload_part_thread = threading.Thread(target=self.upload_part, + args=(1,)) + self.start_thread(upload_part_thread) + + # We announce the upload id. + self.create_upload('my_upload_id') + # The upload_part thread can now proceed, + # now, let's cancel this upload. + self.context.cancel_upload() + + # The upload_part_thread should be finished. + self.join_threads() + + # In a cancelled multipart upload task any subsequent + # call to wait_for_upload_id must raise an UploadCancelledError + with self.assertRaises(UploadCancelledError): + self.context.wait_for_upload_id() + def test_cancel_threads_waiting_for_completion(self): # So we have a thread waiting for the entire upload to complete. arbitrary_waiting_thread = threading.Thread(target=self.wait_for_upload_complete)