Merge pull request #3549 from drocamor/listv2

s3 commands that list buckets will use ListObjectsV2

.changes/next-release/enhancement-s3-89026.json

@@ -0,0 +1,5 @@
+{
+  "type": "enhancement",
+  "category": "s3",
+  "description": "``aws s3`` subcommands that list objects will use ListObjectsV2 instead of ListObjects `#3549 <https://github.com/aws/aws-cli/issues/3549>`__."
+}

awscli/customizations/s3/filegenerator.py

@@ -318,7 +318,7 @@ def list_objects(self, s3_path, dir_op):
             yield self._list_single_object(s3_path)
         else:
             lister = BucketLister(self._client)
-            extra_args = self.request_parameters.get('ListObjects', {})
+            extra_args = self.request_parameters.get('ListObjectsV2', {})
             for key in lister.list_objects(bucket=bucket, prefix=prefix,
                                            page_size=self.page_size,
                                            extra_args=extra_args):

awscli/customizations/s3/subcommands.py

@@ -500,7 +500,7 @@ def _run_main(self, parsed_args, parsed_globals):
 
     def _list_all_objects(self, bucket, key, page_size=None,
                           request_payer=None):
-        paginator = self.client.get_paginator('list_objects')
+        paginator = self.client.get_paginator('list_objects_v2')
         paging_args = {
             'Bucket': bucket, 'Prefix': key, 'Delimiter': '/',
             'PaginationConfig': {'PageSize': page_size}
@@ -548,7 +548,7 @@ def _list_all_buckets(self):
 
     def _list_all_objects_recursive(self, bucket, key, page_size=None,
                                     request_payer=None):
-        paginator = self.client.get_paginator('list_objects')
+        paginator = self.client.get_paginator('list_objects_v2')
         paging_args = {
             'Bucket': bucket, 'Prefix': key,
             'PaginationConfig': {'PageSize': page_size}
@@ -1081,7 +1081,8 @@ def run(self):
     def _get_file_generator_request_parameters_skeleton(self):
         return {
             'HeadObject': {},
-            'ListObjects': {}
+            'ListObjects': {},
+            'ListObjectsV2': {}
         }
 
     def _map_request_payer_params(self, request_parameters):
@@ -1090,8 +1091,8 @@ def _map_request_payer_params(self, request_parameters):
                 'request_payer': self.parameters.get('request_payer')
             }
         )
-        RequestParamsMapper.map_list_objects_params(
-            request_parameters['ListObjects'], {
+        RequestParamsMapper.map_list_objects_v2_params(
+            request_parameters['ListObjectsV2'], {
                 'request_payer': self.parameters.get('request_payer')
             }
         )

awscli/customizations/s3/utils.py

@@ -365,7 +365,7 @@ def list_objects(self, bucket, prefix=None, page_size=None,
         if extra_args is not None:
             kwargs.update(extra_args)
 
-        paginator = self._client.get_paginator('list_objects')
+        paginator = self._client.get_paginator('list_objects_v2')
         pages = paginator.paginate(**kwargs)
         for page in pages:
             contents = page.get('Contents', [])
@@ -480,7 +480,7 @@ def map_delete_object_params(cls, request_params, cli_params):
         cls._set_request_payer_param(request_params, cli_params)
 
     @classmethod
-    def map_list_objects_params(cls, request_params, cli_params):
+    def map_list_objects_v2_params(cls, request_params, cli_params):
         cls._set_request_payer_param(request_params, cli_params)
 
     @classmethod

tests/functional/s3/test_cp_command.py

@@ -133,10 +133,10 @@ def test_operations_used_in_recursive_download(self):
         cmdline = '%s s3://bucket/key.txt %s --recursive' % (
             self.prefix, self.files.rootdir)
         self.run_cmd(cmdline, expected_rc=0)
-        # We called ListObjects but had no objects to download, so
-        # we only have a single ListObjects operation being called.
+        # We called ListObjectsV2 but had no objects to download, so
+        # we only have a single ListObjectsV2 operation being called.
         self.assertEqual(len(self.operations_called), 1, self.operations_called)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
 
     def test_website_redirect_ignore_paramfile(self):
         full_path = self.files.create_file('foo.txt', 'mycontent')
@@ -268,7 +268,7 @@ def test_recursive_glacier_download_with_force_glacier(self):
                   % (self.prefix, self.files.rootdir)
         self.run_cmd(cmdline, expected_rc=0)
         self.assertEqual(len(self.operations_called), 2, self.operations_called)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
         self.assertEqual(self.operations_called[1][0].name, 'GetObject')
 
     def test_recursive_glacier_download_without_force_glacier(self):
@@ -287,7 +287,7 @@ def test_recursive_glacier_download_without_force_glacier(self):
             self.prefix, self.files.rootdir)
         _, stderr, _ = self.run_cmd(cmdline, expected_rc=2)
         self.assertEqual(len(self.operations_called), 1, self.operations_called)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
         self.assertIn('GLACIER', stderr)
 
     def test_warns_on_glacier_incompatible_operation(self):
@@ -784,7 +784,7 @@ def test_recursive_download(self):
         self.run_cmd(cmdline, expected_rc=0)
         self.assert_operations_called(
             [
-                ('ListObjects', {
+                ('ListObjectsV2', {
                     'Bucket': 'mybucket',
                     'Prefix': '',
                     'EncodingType': 'url',
@@ -899,7 +899,7 @@ def test_recursive_copy(self):
         self.run_cmd(cmdline, expected_rc=0)
         self.assert_operations_called(
             [
-                ('ListObjects', {
+                ('ListObjectsV2', {
                     'Bucket': 'sourcebucket',
                     'Prefix': '',
                     'EncodingType': 'url',

tests/functional/s3/test_rb_command.py

@@ -27,7 +27,7 @@ def test_rb_force_empty_bucket(self):
         command = self.prefix + 's3://bucket --force'
         self.run_cmd(command)
         self.assertEqual(len(self.operations_called), 2)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
         self.assertEqual(self.operations_called[1][0].name, 'DeleteBucket')
 
     def test_rb_force_non_empty_bucket(self):
@@ -43,7 +43,7 @@ def test_rb_force_non_empty_bucket(self):
         }, {}, {}]
         self.run_cmd(command)
         self.assertEqual(len(self.operations_called), 3)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
         self.assertEqual(self.operations_called[1][0].name, 'DeleteObject')
         self.assertEqual(self.operations_called[2][0].name, 'DeleteBucket')
 
@@ -59,7 +59,7 @@ def test_rb_force_with_failed_rm(self):
         _, stderr, _ = self.run_cmd(command, expected_rc=255)
         self.assertIn('remove_bucket failed:', stderr)
         self.assertEqual(len(self.operations_called), 1)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
 
     def test_nonzero_exit_if_uri_scheme_not_provided(self):
         command = self.prefix + 'bucket'

tests/functional/s3/test_rm_command.py

@@ -55,7 +55,7 @@ def test_recursive_delete_with_requests(self):
         self.run_cmd(cmdline, expected_rc=0)
         self.assert_operations_called(
             [
-                ('ListObjects', {
+                ('ListObjectsV2', {
                     'Bucket': 'mybucket',
                     'Prefix': '',
                     'EncodingType': 'url',

tests/functional/s3/test_sync_command.py

@@ -32,9 +32,9 @@ def test_website_redirect_ignore_paramfile(self):
         ]
         self.run_cmd(cmdline, expected_rc=0)
 
-        # The only operations we should have called are ListObjects/PutObject.
+        # The only operations we should have called are ListObjectsV2/PutObject.
         self.assertEqual(len(self.operations_called), 2, self.operations_called)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
         self.assertEqual(self.operations_called[1][0].name, 'PutObject')
         # Make sure that the specified web address is used as opposed to the
         # contents of the web address when uploading the object
@@ -90,7 +90,7 @@ def test_glacier_sync_with_force_glacier(self):
             self.prefix, self.files.rootdir)
         self.run_cmd(cmdline, expected_rc=0)
         self.assertEqual(len(self.operations_called), 2, self.operations_called)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
         self.assertEqual(self.operations_called[1][0].name, 'GetObject')
 
     def test_handles_glacier_incompatible_operations(self):
@@ -105,7 +105,7 @@ def test_handles_glacier_incompatible_operations(self):
         # There should not have been a download attempted because the
         # operation was skipped because it is glacier incompatible.
         self.assertEqual(len(self.operations_called), 1)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
         self.assertIn('GLACIER', stderr)
 
     def test_turn_off_glacier_warnings(self):
@@ -120,7 +120,7 @@ def test_turn_off_glacier_warnings(self):
         # There should not have been a download attempted because the
         # operation was skipped because it is glacier incompatible.
         self.assertEqual(len(self.operations_called), 1)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
         self.assertEqual('', stderr)
 
     def test_warning_on_invalid_timestamp(self):
@@ -139,7 +139,7 @@ def test_warning_on_invalid_timestamp(self):
 
         # We should still have put the object
         self.assertEqual(len(self.operations_called), 2, self.operations_called)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
         self.assertEqual(self.operations_called[1][0].name, 'PutObject')
 
     def test_sync_with_delete_on_downloads(self):
@@ -152,9 +152,9 @@ def test_sync_with_delete_on_downloads(self):
         ]
         self.run_cmd(cmdline, expected_rc=0)
 
-        # The only operations we should have called are ListObjects.
+        # The only operations we should have called are ListObjectsV2.
         self.assertEqual(len(self.operations_called), 1, self.operations_called)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
 
         self.assertFalse(os.path.exists(full_path))
 
@@ -185,7 +185,7 @@ def side_effect(_):
         # We should not call PutObject because the file was deleted
         # before we could transfer it
         self.assertEqual(len(self.operations_called), 1, self.operations_called)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
 
     # This test covers the case where an OSError is emitted.
     def test_sync_skips_over_files_deleted_between_listing_and_transfer_oserror(self):
@@ -208,7 +208,7 @@ def side_effect(_):
         # We should not call PutObject because the file was deleted
         # before we could transfer it
         self.assertEqual(len(self.operations_called), 1, self.operations_called)
-        self.assertEqual(self.operations_called[0][0].name, 'ListObjects')
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
 
     def test_request_payer(self):
         cmdline = '%s s3://sourcebucket/ s3://mybucket --request-payer' % (
@@ -234,13 +234,13 @@ def test_request_payer(self):
         self.run_cmd(cmdline, expected_rc=0)
         self.assert_operations_called(
             [
-                ('ListObjects', {
+                ('ListObjectsV2', {
                     'Bucket': 'sourcebucket',
                     'Prefix': '',
                     'EncodingType': 'url',
                     'RequestPayer': 'requester',
                 }),
-                ('ListObjects', {
+                ('ListObjectsV2', {
                     'Bucket': 'mybucket',
                     'Prefix': '',
                     'EncodingType': 'url',
@@ -280,13 +280,13 @@ def test_request_payer_with_deletes(self):
         self.run_cmd(cmdline, expected_rc=0)
         self.assert_operations_called(
             [
-                ('ListObjects', {
+                ('ListObjectsV2', {
                     'Bucket': 'sourcebucket',
                     'Prefix': '',
                     'EncodingType': 'url',
                     'RequestPayer': 'requester',
                 }),
-                ('ListObjects', {
+                ('ListObjectsV2', {
                     'Bucket': 'mybucket',
                     'Prefix': '',
                     'EncodingType': 'url',

tests/integration/customizations/s3/test_plugin.py

@@ -1149,7 +1149,7 @@ def test_ls_non_existent_bucket(self):
         self.assertEqual(p.rc, 255)
         self.assertIn(
             ('An error occurred (NoSuchBucket) when calling the '
-             'ListObjects operation: The specified bucket does not exist'),
+             'ListObjectsV2 operation: The specified bucket does not exist'),
             p.stderr)
         # There should be no stdout if we can't find the bucket.
         self.assertEqual(p.stdout, '')

tests/unit/customizations/s3/test_subcommands.py

@@ -98,14 +98,14 @@ def test_ls_command_for_bucket(self):
                                summarize=False, request_payer=None)
         parsed_globals = mock.Mock()
         ls_command._run_main(parsed_args, parsed_globals)
-        call = self.session.create_client.return_value.list_objects
+        call = self.session.create_client.return_value.list_objects_v2
         paginate = self.session.create_client.return_value.get_paginator\
             .return_value.paginate
         # We should make no operation calls.
         self.assertEqual(call.call_count, 0)
-        # And only a single pagination call to ListObjects.
+        # And only a single pagination call to ListObjectsV2.
         self.session.create_client.return_value.get_paginator.\
-            assert_called_with('list_objects')
+            assert_called_with('list_objects_v2')
         ref_call_args = {'Bucket': u'mybucket', 'Delimiter': '/',
                          'Prefix': u'',
                          'PaginationConfig': {'PageSize': u'5'}}
@@ -160,9 +160,9 @@ def test_ls_with_requester_pays(self):
             .return_value.paginate
         # We should make no operation calls.
         self.assertEqual(call.call_count, 0)
-        # And only a single pagination call to ListObjects.
+        # And only a single pagination call to ListObjectsV2.
         self.session.create_client.return_value.get_paginator.\
-            assert_called_with('list_objects')
+            assert_called_with('list_objects_v2')
         ref_call_args = {
             'Bucket': u'mybucket', 'Delimiter': '/',
             'Prefix': u'', 'PaginationConfig': {'PageSize': '5'},

tests/unit/customizations/s3/test_utils.py

@@ -223,7 +223,7 @@ def setUp(self):
 
     def fake_paginate(self, *args, **kwargs):
         for response in self.responses:
-            self.emitter.emit('after-call.s3.ListObjects', parsed=response)
+            self.emitter.emit('after-call.s3.ListObjectsV2', parsed=response)
         return self.responses
 
     def test_list_objects(self):
@@ -469,7 +469,7 @@ def test_delete_object(self):
 
     def test_list_objects(self):
         params = {}
-        RequestParamsMapper.map_list_objects_params(
+        RequestParamsMapper.map_list_objects_v2_params(
             params, self.cli_params)
         self.assertEqual(params, {'RequestPayer': 'requester'})