Skip to content

Commit e1c4d4a

Browse files
author
Rakshil Modi
committed
Implemented No-overwrite for uploads using cp and mv command
Customizing error message test cases for no_overwrite for upload operations using cp command Code cleanup Test cases for move command Removed unwanted code changes code quality improvement Adding test cases Updating help text Removing unwanted comments fixed identation Updated more test cases
1 parent df9bac5 commit e1c4d4a

File tree

9 files changed

+362
-5
lines changed

9 files changed

+362
-5
lines changed

awscli/customizations/s3/results.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from awscli.customizations.s3.subscribers import OnDoneFilteredSubscriber
2424
from awscli.customizations.s3.utils import WarningResult, human_readable_size
2525
from awscli.customizations.utils import uni_print
26+
from awscli.customizations.s3.utils import create_warning
2627

2728
LOGGER = logging.getLogger(__name__)
2829

@@ -123,7 +124,13 @@ def _on_failure(self, future, e):
123124
if isinstance(e, FatalError):
124125
error_result_cls = ErrorResult
125126
self._result_queue.put(error_result_cls(exception=e))
126-
else:
127+
elif self._is_precondition_failed(e):
128+
LOGGER.debug(
129+
"Warning: Skipping file %s as it already exists on %s",
130+
self._src,
131+
self._dest,
132+
)
133+
else :
127134
self._result_queue.put(
128135
FailureResult(
129136
transfer_type=self._transfer_type,
@@ -132,6 +139,11 @@ def _on_failure(self, future, e):
132139
exception=e,
133140
)
134141
)
142+
143+
def _is_precondition_failed(self, exception):
144+
"""Check if this is a PreconditionFailed error"""
145+
return (hasattr(exception, 'response') and
146+
exception.response.get('Error', {}).get('Code') == 'PreconditionFailed')
135147

136148

137149
class BaseResultHandler:

awscli/customizations/s3/subcommands.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,15 @@
642642
),
643643
}
644644

645+
NO_OVERWRITE = {
646+
'name': 'no-overwrite',
647+
'action': 'store_true',
648+
'help_text': (
649+
"This flag prevents overwriting of files at the destination. With this flag, "
650+
"only files not present at the destination will be transferred."
651+
),
652+
}
653+
645654
TRANSFER_ARGS = [
646655
DRYRUN,
647656
QUIET,
@@ -1057,7 +1066,7 @@ class CpCommand(S3TransferCommand):
10571066
}
10581067
]
10591068
+ TRANSFER_ARGS
1060-
+ [METADATA, COPY_PROPS, METADATA_DIRECTIVE, EXPECTED_SIZE, RECURSIVE]
1069+
+ [METADATA, COPY_PROPS, METADATA_DIRECTIVE, EXPECTED_SIZE, RECURSIVE, NO_OVERWRITE]
10611070
)
10621071

10631072

@@ -1081,6 +1090,7 @@ class MvCommand(S3TransferCommand):
10811090
METADATA_DIRECTIVE,
10821091
RECURSIVE,
10831092
VALIDATE_SAME_S3_PATHS,
1093+
NO_OVERWRITE,
10841094
]
10851095
)
10861096

awscli/customizations/s3/utils.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,7 @@ def map_put_object_params(cls, request_params, cli_params):
489489
cls._set_sse_c_request_params(request_params, cli_params)
490490
cls._set_request_payer_param(request_params, cli_params)
491491
cls._set_checksum_algorithm_param(request_params, cli_params)
492+
cls._set_no_overwrite_param(request_params, cli_params)
492493

493494
@classmethod
494495
def map_get_object_params(cls, request_params, cli_params):
@@ -557,6 +558,12 @@ def map_delete_object_params(cls, request_params, cli_params):
557558
@classmethod
558559
def map_list_objects_v2_params(cls, request_params, cli_params):
559560
cls._set_request_payer_param(request_params, cli_params)
561+
562+
@classmethod
563+
def _set_no_overwrite_param(cls, request_params, cli_params):
564+
"""Map No overwrite header with IfNoneMatch"""
565+
if cli_params.get('no_overwrite'):
566+
request_params['IfNoneMatch'] = "*"
560567

561568
@classmethod
562569
def _set_request_payer_param(cls, request_params, cli_params):

awscli/s3transfer/manager.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ class TransferManager:
195195
+ [
196196
'ChecksumType',
197197
'MpuObjectSize',
198+
'IfNoneMatch',
198199
]
199200
+ FULL_OBJECT_CHECKSUM_ARGS
200201
)

awscli/s3transfer/upload.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ class UploadSubmissionTask(SubmissionTask):
515515

516516
PUT_OBJECT_BLOCKLIST = ["ChecksumType", "MpuObjectSize"]
517517

518-
CREATE_MULTIPART_BLOCKLIST = FULL_OBJECT_CHECKSUM_ARGS + ["MpuObjectSize"]
518+
CREATE_MULTIPART_BLOCKLIST = FULL_OBJECT_CHECKSUM_ARGS + ["MpuObjectSize", "IfNoneMatch"]
519519

520520
UPLOAD_PART_ARGS = [
521521
'ChecksumAlgorithm',
@@ -534,6 +534,7 @@ class UploadSubmissionTask(SubmissionTask):
534534
'ExpectedBucketOwner',
535535
'ChecksumType',
536536
'MpuObjectSize',
537+
'IfNoneMatch',
537538
] + FULL_OBJECT_CHECKSUM_ARGS
538539

539540
def _get_upload_input_manager_cls(self, transfer_future):

tests/functional/s3/test_cp_command.py

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,98 @@ def test_operations_used_in_recursive_download(self):
295295
len(self.operations_called), 1, self.operations_called
296296
)
297297
self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
298+
299+
def test_for_no_overwrite_flag_when_object_not_exists_on_target(self):
300+
"""Testing when object with different key is successsfully uploaded using no-overwrite"""
301+
full_path = self.files.create_file('foo.txt', 'mycontent')
302+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
303+
self.parsed_responses = [
304+
{'ETag': '"c8afdb36c52cf4727836669019e69222"'}
305+
]
306+
self.run_cmd(cmdline, expected_rc=0)
307+
# Verify putObject was called
308+
self.assertEqual(len(self.operations_called), 1)
309+
self.assertEqual(self.operations_called[0][0].name, 'PutObject')
310+
# Verify the IfNoneMatch condition was set in the request
311+
self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
312+
313+
def test_for_no_overwrite_flag_when_object_exists_on_target(self):
314+
"""Testing when object already exists using no-overwrite"""
315+
full_path = self.files.create_file('foo.txt', 'mycontent')
316+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
317+
# Set up the response to simulate a PreconditionFailed error
318+
self.http_response.status_code = 412
319+
self.parsed_responses = [
320+
{
321+
'Error': {
322+
'Code': 'PreconditionFailed',
323+
'Message': 'At least one of the pre-conditions you specified did not hold',
324+
'Condition': 'If-None-match'
325+
}
326+
}
327+
]
328+
self.run_cmd(cmdline, expected_rc=0)
329+
# Verify PutObject was attempted with IfNoneMatch
330+
self.assertEqual(len(self.operations_called), 1)
331+
self.assertEqual(self.operations_called[0][0].name, 'PutObject')
332+
self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
298333

334+
def test_for_no_overwrite_flag_multipart_upload_when_object_not_exists_on_target(self):
335+
"""Testing multipart upload with no-overwrite flag when object doesn't exist"""
336+
# Create a large file that will trigger multipart upload
337+
full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
338+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
339+
340+
# Set up responses for multipart upload
341+
self.parsed_responses = [
342+
{'UploadId': 'foo'}, # CreateMultipartUpload response
343+
{'ETag': '"foo-1"'}, # UploadPart response
344+
{'ETag': '"foo-2"'}, # UploadPart response
345+
{} # CompleteMultipartUpload response
346+
]
347+
self.run_cmd(cmdline, expected_rc=0)
348+
# Verify all multipart operations were called
349+
self.assertEqual(len(self.operations_called), 4)
350+
self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
351+
self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
352+
self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
353+
self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
354+
# Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
355+
self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
356+
357+
def test_for_no_overwrite_flag_multipart_upload_when_object_exists_on_target(self):
358+
"""Testing multipart upload with no-overwrite flag when object already exist"""
359+
# Create a large file that will trigger multipart upload
360+
full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
361+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
362+
# Set up responses for multipart upload
363+
self.parsed_responses = [
364+
{'UploadId': 'foo'}, # CreateMultipartUpload response
365+
{'ETag': '"foo-1"'}, # UploadPart response
366+
{'ETag': '"foo-2"'}, # UploadPart response
367+
{
368+
'Error': {
369+
'Code': 'PreconditionFailed',
370+
'Message': 'At least one of the pre-conditions you specified did not hold',
371+
'Condition': 'If-None-match'
372+
}
373+
}, # PreconditionFailed error for CompleteMultipart Upload
374+
{} # AbortMultipartUpload response
375+
]
376+
# Checking for success as file is skipped
377+
self.run_cmd(cmdline, expected_rc=0)
378+
# Set up the response to simulate a PreconditionFailed error
379+
self.http_response.status_code = 412
380+
# Verify all multipart operations were called
381+
self.assertEqual(len(self.operations_called), 5)
382+
self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
383+
self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
384+
self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
385+
self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
386+
self.assertEqual(self.operations_called[4][0].name, 'AbortMultipartUpload')
387+
# Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
388+
self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
389+
299390
def test_dryrun_download(self):
300391
self.parsed_responses = [self.head_object_response()]
301392
target = self.files.full_path('file.txt')

tests/functional/s3/test_mv_command.py

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,8 +315,102 @@ def test_download_with_checksum_mode_crc32(self):
315315
self.assertEqual(
316316
self.operations_called[1][1]['ChecksumMode'], 'ENABLED'
317317
)
318-
319-
318+
319+
def test_mv_no_overwrite_flag_when_object_not_exists_on_target(self):
320+
"""Testing when object doesnt exist using no-overwrite"""
321+
full_path = self.files.create_file('foo.txt', 'contents')
322+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
323+
self.run_cmd(cmdline, expected_rc=0)
324+
# Verify putObject was called
325+
self.assertEqual(len(self.operations_called),1)
326+
self.assertEqual(self.operations_called[0][0].name, 'PutObject')
327+
# Verify the IfNoneMatch condition was set in the request
328+
self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
329+
# Verify source file was deleted (move operation)
330+
self.assertFalse(os.path.exists(full_path))
331+
332+
def test_mv_no_overwrite_flag_when_object_exists_on_target(self):
333+
"""Testing when object already exists using no-overwrite"""
334+
full_path = self.files.create_file('foo.txt', 'mycontent')
335+
cmdline = f'{self.prefix} {full_path} s3://bucket/foo.txt --no-overwrite'
336+
# Set up the response to simulate a PreconditionFailed error
337+
self.http_response.status_code = 412
338+
self.parsed_responses = [
339+
{
340+
'Error': {
341+
'Code': 'PreconditionFailed',
342+
'Message': 'At least one of the pre-conditions you specified did not hold',
343+
'Condition': 'If-None-match'
344+
}
345+
}
346+
]
347+
self.run_cmd(cmdline, expected_rc=0)
348+
# Verify PutObject was attempted with IfNoneMatch
349+
self.assertEqual(len(self.operations_called), 1)
350+
self.assertEqual(self.operations_called[0][0].name, 'PutObject')
351+
self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
352+
# Verify source file was not deleted
353+
self.assertTrue(os.path.exists(full_path))
354+
355+
def test_mv_no_overwrite_flag_multipart_upload_when_object_not_exists_on_target(self):
356+
"""Testing multipart upload with no-overwrite flag when object doesn't exist"""
357+
# Create a large file that will trigger multipart upload
358+
full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
359+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
360+
# Set up responses for multipart upload
361+
self.parsed_responses = [
362+
{'UploadId': 'foo'}, # CreateMultipartUpload response
363+
{'ETag': '"foo-1"'}, # UploadPart response
364+
{'ETag': '"foo-2"'}, # UploadPart response
365+
{} # CompleteMultipartUpload response
366+
]
367+
self.run_cmd(cmdline, expected_rc=0)
368+
# Verify all multipart operations were called
369+
self.assertEqual(len(self.operations_called), 4)
370+
self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
371+
self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
372+
self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
373+
self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
374+
# Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
375+
self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
376+
# Verify source file was deleted (successful move operation)
377+
self.assertFalse(os.path.exists(full_path))
378+
379+
def test_mv_no_overwrite_flag_multipart_upload_when_object_exists_on_target(self):
380+
"""Testing multipart upload with no-overwrite flag when object exist at the target"""
381+
# Create a large file that will trigger multipart upload
382+
full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
383+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
384+
# Set up responses for multipart upload
385+
self.parsed_responses = [
386+
{'UploadId': 'foo'}, # CreateMultipartUpload response
387+
{'ETag': '"foo-1"'}, # UploadPart response
388+
{'ETag': '"foo-2"'}, # UploadPart response
389+
{
390+
'Error': {
391+
'Code': 'PreconditionFailed',
392+
'Message': 'At least one of the pre-conditions you specified did not hold',
393+
'Condition': 'If-None-match'
394+
}
395+
}, # CompleteMultipartUpload response
396+
{} # Abort Multipart
397+
]
398+
self.run_cmd(cmdline, expected_rc=0)
399+
# Set up the response to simulate a PreconditionFailed error
400+
self.http_response.status_code = 412
401+
# Verify all multipart operations were called
402+
self.assertEqual(len(self.operations_called), 5)
403+
self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
404+
self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
405+
self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
406+
self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
407+
self.assertEqual(self.operations_called[4][0].name, 'AbortMultipartUpload')
408+
# Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
409+
self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
410+
# Verify source file was not deleted (failed move operation due to PreconditionFailed)
411+
self.assertTrue(os.path.exists(full_path))
412+
413+
320414
class TestMvWithCRTClient(BaseCRTTransferClientTest):
321415
def test_upload_move_using_crt_client(self):
322416
filename = self.files.create_file('myfile', 'mycontent')

0 commit comments

Comments
 (0)