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 2 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
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
12 changes: 9 additions & 3 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 @@ -1095,6 +1096,11 @@ def _map_request_payer_params(self, request_parameters):
'request_payer': self.parameters.get('request_payer')
}
)
RequestParamsMapper.map_list_objects_params(
drocamor marked this conversation as resolved.
Show resolved Hide resolved
request_parameters['ListObjectsV2'], {
'request_payer': self.parameters.get('request_payer')
}
)

def _map_sse_c_params(self, request_parameters, paths_type):
# SSE-C may be neaded for HeadObject for copies/downloads/deletes
Expand Down
2 changes: 1 addition & 1 deletion 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
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
8 changes: 4 additions & 4 deletions tests/unit/customizations/s3/test_subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ def test_ls_command_for_bucket(self):
.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
2 changes: 1 addition & 1 deletion 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