Merge pull request #9609 from rakshil14-2/no_overwrite_for_sync_operation

Implementing no-overwrite for sync and download

Author: kdaily

awscli/customizations/s3/s3handler.py

@@ -438,7 +438,36 @@ def _get_fileout(self, fileinfo):
         return fileinfo.dest
 
     def _get_warning_handlers(self):
-        return [self._warn_glacier, self._warn_parent_reference]
+        return [
+            self._warn_glacier,
+            self._warn_parent_reference,
+            self._warn_if_file_exists_with_no_overwrite,
+        ]
+
+    def _warn_if_file_exists_with_no_overwrite(self, fileinfo):
+        """
+        Warning handler to skip downloads when no-overwrite is set and local file exists.
+
+        This method prevents overwriting existing local files during S3 download operations
+        when the --no-overwrite flag is specified. It checks if the destination file already
+        exists on the local filesystem and skips the download if found.
+
+        :type fileinfo: FileInfo
+        :param fileinfo: The FileInfo object containing transfer details
+
+        :rtype: bool
+        :returns: True if the file should be skipped (exists and no-overwrite is set),
+                False if the download should proceed
+        """
+        if not self._cli_params.get('no_overwrite'):
+            return False
+        fileout = self._get_fileout(fileinfo)
+        if os.path.exists(fileout):
+            LOGGER.debug(
+                f"warning: skipping {fileinfo.src} -> {fileinfo.dest}, file exists at destination"
+            )
+            return True
+        return False
 
     def _format_src_dest(self, fileinfo):
         src = self._format_s3_path(fileinfo.src)
@@ -512,9 +541,7 @@ def _warn_if_zero_byte_file_exists_with_no_overwrite(self, fileinfo):
         try:
             client.head_object(Bucket=bucket, Key=key)
             LOGGER.debug(
-                "Warning: Skipping file %s as it already exists on %s",
-                fileinfo.src,
-                fileinfo.dest,
+                f"warning: skipping {fileinfo.src} -> {fileinfo.dest}, file exists at destination"
             )
             return True
         except ClientError as e:

awscli/customizations/s3/subcommands.py

@@ -1143,7 +1143,7 @@ class SyncCommand(S3TransferCommand):
             }
         ]
         + TRANSFER_ARGS
-        + [METADATA, COPY_PROPS, METADATA_DIRECTIVE]
+        + [METADATA, COPY_PROPS, METADATA_DIRECTIVE, NO_OVERWRITE]
     )
 
 
@@ -1314,7 +1314,6 @@ def choose_sync_strategies(self):
                     sync_type = override_sync_strategy.sync_type
                     sync_type += '_sync_strategy'
                     sync_strategies[sync_type] = override_sync_strategy
-
         return sync_strategies
 
     def run(self):
@@ -1401,7 +1400,8 @@ def run(self):
             self._client, self._source_client, self.parameters
         )
 
-        s3_transfer_handler = S3TransferHandlerFactory(self.parameters)(
+        params = self._get_s3_handler_params()
+        s3_transfer_handler = S3TransferHandlerFactory(params)(
             self._transfer_manager, result_queue
         )
 
@@ -1510,6 +1510,16 @@ def _map_sse_c_params(self, request_parameters, paths_type):
                 },
             )
 
+    def _get_s3_handler_params(self):
+        """
+        Removing no-overwrite params from sync since file to
+        be synced are already separated out using sync strategy
+        """
+        params = self.parameters.copy()
+        if self.cmd == 'sync':
+            params.pop('no_overwrite', None)
+        return params
+
 
 # TODO: This class is fairly quirky in the sense that it is both a builder
 #  and a data object. In the future we should make the following refactorings
@@ -1573,6 +1583,7 @@ def add_paths(self, paths):
         elif len(paths) == 1:
             self.parameters['dest'] = paths[0]
         self._validate_streaming_paths()
+        self._validate_no_overwrite_for_download_streaming()
         self._validate_path_args()
         self._validate_sse_c_args()
         self._validate_not_s3_express_bucket_for_sync()
@@ -1824,3 +1835,20 @@ def _validate_sse_c_copy_source_for_paths(self):
                     '--sse-c-copy-source is only supported for '
                     'copy operations.'
                 )
+
+    def _validate_no_overwrite_for_download_streaming(self):
+        """
+        Validates that no-overwrite parameter is not used with streaming downloads.
+
+        Raises:
+            ParamValidationError: If no-overwrite is specified with a streaming download.
+        """
+        if (
+            self.parameters['is_stream']
+            and self.parameters.get('no_overwrite')
+            and self.parameters['dest'] == '-'
+        ):
+            raise ParamValidationError(
+                "--no-overwrite parameter is not supported for "
+                "streaming downloads"
+            )

awscli/customizations/s3/syncstrategy/nooverwrite.py

@@ -0,0 +1,34 @@
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+import logging
+
+from awscli.customizations.s3.subcommands import NO_OVERWRITE
+from awscli.customizations.s3.syncstrategy.base import BaseSync
+
+LOG = logging.getLogger(__name__)
+
+
+class NoOverwriteSync(BaseSync):
+    """Sync strategy that prevents overwriting of existing files at the destination.
+    This strategy is used only for files that exist at both source and destination
+    (file_at_src_and_dest_sync_strategy). It always returns False to prevent any
+    overwriting of existing files, regardless of size or modification time differences.
+    """
+
+    ARGUMENT = NO_OVERWRITE
+
+    def determine_should_sync(self, src_file, dest_file):
+        LOG.debug(
+            f"warning: skipping {src_file.src} -> {src_file.dest}, file exists at destination"
+        )
+        return False

awscli/customizations/s3/syncstrategy/register.py

@@ -14,6 +14,7 @@
 from awscli.customizations.s3.syncstrategy.exacttimestamps import (
     ExactTimestampsSync,
 )
+from awscli.customizations.s3.syncstrategy.nooverwrite import NoOverwriteSync
 from awscli.customizations.s3.syncstrategy.sizeonly import SizeOnlySync
 
 
@@ -48,4 +49,7 @@ def register_sync_strategies(command_table, session, **kwargs):
     # Register the delete sync strategy.
     register_sync_strategy(session, DeleteSync, 'file_not_at_src')
 
+    # Register the noOverwrite sync strategy
+    register_sync_strategy(session, NoOverwriteSync, 'file_at_src_and_dest')
+
     # Register additional sync strategies here...

tests/functional/s3/test_cp_command.py

@@ -315,13 +315,7 @@ def test_no_overwrite_flag_when_object_exists_on_target(self):
         # Set up the response to simulate a PreconditionFailed error
         self.http_response.status_code = 412
         self.parsed_responses = [
-            {
-                'Error': {
-                    'Code': 'PreconditionFailed',
-                    'Message': 'At least one of the pre-conditions you specified did not hold',
-                    'Condition': 'If-None-Match',
-                }
-            }
+            self.precondition_failed_error_response(),
         ]
         self.run_cmd(cmdline, expected_rc=0)
         # Verify PutObject was attempted with IfNoneMatch
@@ -367,13 +361,7 @@ def test_no_overwrite_flag_multipart_upload_when_object_exists_on_target(
             {'UploadId': 'foo'},  # CreateMultipartUpload response
             {'ETag': '"foo-1"'},  # UploadPart response
             {'ETag': '"foo-2"'},  # UploadPart response
-            {
-                'Error': {
-                    'Code': 'PreconditionFailed',
-                    'Message': 'At least one of the pre-conditions you specified did not hold',
-                    'Condition': 'If-None-Match',
-                }
-            },  # PreconditionFailed error for CompleteMultipart Upload
+            self.precondition_failed_error_response(),  # PreconditionFailed error for CompleteMultipart Upload
             {},  # AbortMultipartUpload response
         ]
         # Checking for success as file is skipped
@@ -550,6 +538,40 @@ def test_no_overwrite_flag_on_copy_when_large_object_does_not_exist_on_target(
         # Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
         self.assertEqual(self.operations_called[5][1]['IfNoneMatch'], '*')
 
+    def test_no_overwrite_flag_on_download_when_single_object_already_exists_at_target(
+        self,
+    ):
+        full_path = self.files.create_file('foo.txt', 'existing content')
+        cmdline = (
+            f'{self.prefix} s3://bucket/foo.txt {full_path} --no-overwrite'
+        )
+        self.parsed_responses = [
+            self.head_object_response(),
+        ]
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 1)
+        self.assertEqual(self.operations_called[0][0].name, 'HeadObject')
+        with open(full_path) as f:
+            self.assertEqual(f.read(), 'existing content')
+
+    def test_no_overwrite_flag_on_download_when_single_object_does_not_exist_at_target(
+        self,
+    ):
+        full_path = self.files.full_path('foo.txt')
+        cmdline = (
+            f'{self.prefix} s3://bucket/foo.txt {full_path} --no-overwrite'
+        )
+        self.parsed_responses = [
+            self.head_object_response(),
+            self.get_object_response(),
+        ]
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 2)
+        self.assertEqual(self.operations_called[0][0].name, 'HeadObject')
+        self.assertEqual(self.operations_called[1][0].name, 'GetObject')
+        with open(full_path) as f:
+            self.assertEqual(f.read(), 'foo')
+
     def test_dryrun_download(self):
         self.parsed_responses = [self.head_object_response()]
         target = self.files.full_path('file.txt')
@@ -1428,6 +1450,15 @@ def test_streaming_download_error(self):
         )
         self.assertIn(error_message, stderr)
 
+    def test_no_overwrite_cannot_be_used_with_streaming_download(self):
+        command = "s3 cp s3://bucket/streaming.txt - --no-overwrite"
+        _, stderr, _ = self.run_cmd(command, expected_rc=252)
+        error_message = (
+            "--no-overwrite parameter is not supported for "
+            "streaming downloads"
+        )
+        self.assertIn(error_message, stderr)
+
 
 class TestCpCommandWithRequesterPayer(BaseCPCommandTest):
     def setUp(self):

tests/functional/s3/test_mv_command.py

@@ -336,13 +336,7 @@ def test_mv_no_overwrite_flag_when_object_exists_on_target(self):
         # Set up the response to simulate a PreconditionFailed error
         self.http_response.status_code = 412
         self.parsed_responses = [
-            {
-                'Error': {
-                    'Code': 'PreconditionFailed',
-                    'Message': 'At least one of the pre-conditions you specified did not hold',
-                    'Condition': 'If-None-Match',
-                }
-            }
+            self.precondition_failed_error_response(),
         ]
         self.run_cmd(cmdline, expected_rc=0)
         # Verify PutObject was attempted with IfNoneMatch
@@ -392,13 +386,7 @@ def test_mv_no_overwrite_flag_multipart_upload_when_object_exists_on_target(
             {'UploadId': 'foo'},  # CreateMultipartUpload response
             {'ETag': '"foo-1"'},  # UploadPart response
             {'ETag': '"foo-2"'},  # UploadPart response
-            {
-                'Error': {
-                    'Code': 'PreconditionFailed',
-                    'Message': 'At least one of the pre-conditions you specified did not hold',
-                    'Condition': 'If-None-Match',
-                }
-            },  # CompleteMultipartUpload response
+            self.precondition_failed_error_response(),  # CompleteMultipartUpload response
             {},  # Abort Multipart
         ]
         self.run_cmd(cmdline, expected_rc=0)
@@ -575,6 +563,42 @@ def test_mv_no_overwrite_flag_when_large_object_does_not_exist_on_target(
         )
         self.assertEqual(self.operations_called[6][0].name, 'DeleteObject')
 
+    def test_no_overwrite_flag_on_mv_download_when_single_object_exists_at_target(
+        self,
+    ):
+        full_path = self.files.create_file('foo.txt', 'existing content')
+        cmdline = (
+            f'{self.prefix} s3://bucket/foo.txt {full_path} --no-overwrite'
+        )
+        self.parsed_responses = [
+            self.head_object_response(),
+        ]
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 1)
+        self.assertEqual(self.operations_called[0][0].name, 'HeadObject')
+        with open(full_path) as f:
+            self.assertEqual(f.read(), 'existing content')
+
+    def test_no_overwrite_flag_on_mv_download_when_single_object_does_not_exist_at_target(
+        self,
+    ):
+        full_path = self.files.full_path('foo.txt')
+        cmdline = (
+            f'{self.prefix} s3://bucket/foo.txt {full_path} --no-overwrite'
+        )
+        self.parsed_responses = [
+            self.head_object_response(),
+            self.get_object_response(),
+            self.delete_object_response(),
+        ]
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 3)
+        self.assertEqual(self.operations_called[0][0].name, 'HeadObject')
+        self.assertEqual(self.operations_called[1][0].name, 'GetObject')
+        self.assertEqual(self.operations_called[2][0].name, 'DeleteObject')
+        with open(full_path) as f:
+            self.assertEqual(f.read(), 'foo')
+
 
 class TestMvWithCRTClient(BaseCRTTransferClientTest):
     def test_upload_move_using_crt_client(self):