diff --git a/.changes/next-release/enhancement-s3-89026.json b/.changes/next-release/enhancement-s3-89026.json new file mode 100644 index 000000000000..b322d7d52b78 --- /dev/null +++ b/.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 `__." +} diff --git a/awscli/customizations/s3/filegenerator.py b/awscli/customizations/s3/filegenerator.py index 3d802aba15ce..63a66996811e 100644 --- a/awscli/customizations/s3/filegenerator.py +++ b/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): diff --git a/awscli/customizations/s3/subcommands.py b/awscli/customizations/s3/subcommands.py index 946acf377611..691261343197 100644 --- a/awscli/customizations/s3/subcommands.py +++ b/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') } ) diff --git a/awscli/customizations/s3/utils.py b/awscli/customizations/s3/utils.py index b46c3d0019cc..dbbc35271f0c 100644 --- a/awscli/customizations/s3/utils.py +++ b/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 diff --git a/tests/functional/s3/test_cp_command.py b/tests/functional/s3/test_cp_command.py index 94218c59501a..751e046dea95 100644 --- a/tests/functional/s3/test_cp_command.py +++ b/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', diff --git a/tests/functional/s3/test_rb_command.py b/tests/functional/s3/test_rb_command.py index 14565ac8e360..06d82387bcf8 100644 --- a/tests/functional/s3/test_rb_command.py +++ b/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' diff --git a/tests/functional/s3/test_rm_command.py b/tests/functional/s3/test_rm_command.py index bd97ad5ef0df..b5358f3f8432 100644 --- a/tests/functional/s3/test_rm_command.py +++ b/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', diff --git a/tests/functional/s3/test_sync_command.py b/tests/functional/s3/test_sync_command.py index 7c9f8461414e..893841d78553 100644 --- a/tests/functional/s3/test_sync_command.py +++ b/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', diff --git a/tests/integration/customizations/s3/test_plugin.py b/tests/integration/customizations/s3/test_plugin.py index a93ea8ac803c..55ebbe1f7561 100644 --- a/tests/integration/customizations/s3/test_plugin.py +++ b/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, '') diff --git a/tests/unit/customizations/s3/test_subcommands.py b/tests/unit/customizations/s3/test_subcommands.py index 1b80d03a66c5..a4418a79dae4 100644 --- a/tests/unit/customizations/s3/test_subcommands.py +++ b/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'}, diff --git a/tests/unit/customizations/s3/test_utils.py b/tests/unit/customizations/s3/test_utils.py index 2860dc75582b..3f3eb03b930a 100644 --- a/tests/unit/customizations/s3/test_utils.py +++ b/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'})