Added no overwrite for download operation

handled error during download streaming streaming

Author: Rakshil Modi

awscli/customizations/s3/subcommands.py

@@ -1603,6 +1603,7 @@ def _validate_streaming_paths(self):
             self.parameters['is_stream'] = True
             self.parameters['dir_op'] = False
             self.parameters['only_show_errors'] = True
+            self._validate_streaming_no_overwrite_for_download_parameter()
 
     def _validate_path_args(self):
         # If we're using a mv command, you can't copy the object onto itself.
@@ -1824,3 +1825,22 @@ def _validate_sse_c_copy_source_for_paths(self):
                     '--sse-c-copy-source is only supported for '
                     'copy operations.'
                 )
+
+    def _validate_streaming_no_overwrite_for_download_parameter(self):
+        """
+        Validates that no-overwrite parameter is not used with streaming downloads.
+
+        When downloading from S3 to stdout (streaming download), the no-overwrite
+        parameter doesn't make sense since stdout is not a file that can be checked
+        for existence. This method checks for this invalid combination and raises
+        an appropriate error.
+
+        Raises:
+            ParamValidationError: If no-overwrite is specified with a streaming download.
+        """
+        params = self.parameters
+        if params['paths_type'] == 's3local' and params.get('no_overwrite'):
+            raise ParamValidationError(
+                "--no-overwrite parameter is not supported for "
+                "streaming downloads"
+            )

tests/functional/s3/test_cp_command.py

@@ -550,6 +550,74 @@ 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_small_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_small_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_no_overwrite_flag_on_download_when_large_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(ContentLength=10 * (1024**2)),
+        ]
+        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_large_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(ContentLength=10 * (1024**2)),
+            self.get_object_response(),
+            self.get_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, 'GetObject')
+
     def test_dryrun_download(self):
         self.parsed_responses = [self.head_object_response()]
         target = self.files.full_path('file.txt')

tests/functional/s3/test_mv_command.py

@@ -575,6 +575,78 @@ 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_small_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_small_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')
+
+    def test_no_overwrite_flag_on_mv_download_when_large_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(ContentLength=10 * (1024**2)),
+        ]
+        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_large_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(ContentLength=10 * (1024**2)),
+            self.get_object_response(),
+            self.get_object_response(),
+            self.delete_object_response(),
+        ]
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 4)
+        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, 'GetObject')
+        self.assertEqual(self.operations_called[3][0].name, 'DeleteObject')
+
 
 class TestMvWithCRTClient(BaseCRTTransferClientTest):
     def test_upload_move_using_crt_client(self):

tests/unit/customizations/s3/test_subcommands.py

@@ -524,10 +524,21 @@ def test_validate_streaming_paths_download(self):
         paths = ['s3://bucket/key', '-']
         cmd_params = CommandParameters('cp', {}, '')
         cmd_params.add_paths(paths)
+        print(" Paramertes %s", cmd_params.parameters)
         self.assertTrue(cmd_params.parameters['is_stream'])
         self.assertTrue(cmd_params.parameters['only_show_errors'])
         self.assertFalse(cmd_params.parameters['dir_op'])
 
+    def test_validate_streaming_paths_with_no_overwrite(self):
+        paths = ['s3://bucket/key', '-']
+        cmd_params = CommandParameters('cp', {'no_overwrite': True}, '')
+        with self.assertRaises(ParamValidationError) as cm:
+            cmd_params.add_paths(paths)
+            self.assertIn(
+                '--no-overwrite parameter is not supported for streaming downloads',
+                cm.msg,
+            )
+
     def test_validate_no_streaming_paths(self):
         paths = [self.file_creator.rootdir, 's3://bucket']
         cmd_params = CommandParameters('cp', {}, '')