Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

s3 commands that list buckets will use ListObjectsV2 #3549

Merged
merged 5 commits into from
Sep 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/next-release/enhancement-s3-89026.json
Original file line number Diff line number Diff line change
@@ -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>`__."
}
2 changes: 1 addition & 1 deletion awscli/customizations/s3/filegenerator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
11 changes: 6 additions & 5 deletions awscli/customizations/s3/subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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):
Expand All @@ -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')
}
)
Expand Down
4 changes: 2 additions & 2 deletions awscli/customizations/s3/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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', [])
Expand Down Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions tests/functional/s3/test_cp_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
6 changes: 3 additions & 3 deletions tests/functional/s3/test_rb_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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')

Expand All @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/s3/test_rm_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
28 changes: 14 additions & 14 deletions tests/functional/s3/test_sync_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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))

Expand Down Expand Up @@ -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):
Expand All @@ -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' % (
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/customizations/s3/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, '')
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/customizations/s3/test_subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
drocamor marked this conversation as resolved.
Show resolved Hide resolved
# 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'}}
Expand Down Expand Up @@ -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'},
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/customizations/s3/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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'})

Expand Down