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

Only display --page-size when operation is paginated #956

Closed
wants to merge 2 commits into from
Closed
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
2 changes: 0 additions & 2 deletions awscli/clidriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,6 @@ def invoke(self, operation_object, parameters, parsed_globals):
endpoint_url=parsed_globals.endpoint_url,
verify=parsed_globals.verify_ssl)
if operation_object.can_paginate and parsed_globals.paginate:
if parsed_globals.page_size:
parameters['page_size'] = parsed_globals.page_size
pages = operation_object.paginate(endpoint, **parameters)
self._display_response(operation_object, pages,
parsed_globals)
Expand Down
6 changes: 6 additions & 0 deletions awscli/customizations/paginate.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@
be provided in the output that you can use to resume pagination.
"""

PAGE_SIZE_HELP = """
<p>The size of each page.<p>
"""


def register_pagination(event_handlers):
event_handlers.register('building-argument-table',
Expand Down Expand Up @@ -78,6 +82,8 @@ def unify_paging_params(argument_table, operation, event_name, **kwargs):
raise TypeError(('Unsupported pagination type {0} for operation {1}'
' and parameter {2}').format(type_name, operation.name,
operation.pagination['limit_key']))
argument_table['page-size'] = PageArgument('page-size', PAGE_SIZE_HELP,
operation, parse_type=type_name)

argument_table['max-items'] = PageArgument('max-items', MAX_ITEMS_HELP,
operation, parse_type=type_name)
Expand Down
23 changes: 15 additions & 8 deletions awscli/customizations/s3/subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,17 @@
'under these conditions may result in a failed upload. '
'due to too many parts in upload.')}


PAGE_SIZE = {'name': 'page-size', 'help_text': 'The size of each page.',
'cli_type_name': 'integer'}


TRANSFER_ARGS = [DRYRUN, QUIET, RECURSIVE, INCLUDE, EXCLUDE, ACL,
FOLLOW_SYMLINKS, NO_FOLLOW_SYMLINKS, NO_GUESS_MIME_TYPE,
SSE, STORAGE_CLASS, GRANTS, WEBSITE_REDIRECT, CONTENT_TYPE,
CACHE_CONTROL, CONTENT_DISPOSITION, CONTENT_ENCODING,
CONTENT_LANGUAGE, EXPIRES, SOURCE_REGION, ONLY_SHOW_ERRORS]
CONTENT_LANGUAGE, EXPIRES, SOURCE_REGION, ONLY_SHOW_ERRORS,
PAGE_SIZE]


def get_endpoint(service, region, endpoint_url, verify):
Expand All @@ -232,7 +238,8 @@ class ListCommand(S3Command):
"is ignored for this command.")
USAGE = "<S3Path> or NONE"
ARG_TABLE = [{'name': 'paths', 'nargs': '?', 'default': 's3://',
'positional_arg': True, 'synopsis': USAGE}, RECURSIVE]
'positional_arg': True, 'synopsis': USAGE}, RECURSIVE,
PAGE_SIZE]
EXAMPLES = BasicCommand.FROM_FILE('s3/ls.rst')

def _run_main(self, parsed_args, parsed_globals):
Expand All @@ -246,9 +253,9 @@ def _run_main(self, parsed_args, parsed_globals):
elif parsed_args.dir_op:
# Then --recursive was specified.
self._list_all_objects_recursive(bucket, key,
parsed_globals.page_size)
parsed_args.page_size)
else:
self._list_all_objects(bucket, key, parsed_globals.page_size)
self._list_all_objects(bucket, key, parsed_args.page_size)
return 0

def _list_all_objects(self, bucket, key, page_size=None):
Expand Down Expand Up @@ -370,7 +377,7 @@ def _run_main(self, parsed_args, parsed_globals):
cmd_params.add_region(parsed_globals)
cmd_params.add_endpoint_url(parsed_globals)
cmd_params.add_verify_ssl(parsed_globals)
cmd_params.add_page_size(parsed_globals)
cmd_params.add_page_size(parsed_args)
cmd_params.add_paths(parsed_args.paths)
cmd_params.check_force(parsed_globals)
cmd = CommandArchitecture(self._session, self.NAME,
Expand Down Expand Up @@ -428,7 +435,7 @@ class RmCommand(S3TransferCommand):
USAGE = "<S3Path>"
ARG_TABLE = [{'name': 'paths', 'nargs': 1, 'positional_arg': True,
'synopsis': USAGE}, DRYRUN, QUIET, RECURSIVE, INCLUDE,
EXCLUDE, ONLY_SHOW_ERRORS]
EXCLUDE, ONLY_SHOW_ERRORS, PAGE_SIZE]
EXAMPLES = BasicCommand.FROM_FILE('s3/rm.rst')


Expand Down Expand Up @@ -857,5 +864,5 @@ def add_endpoint_url(self, parsed_globals):
def add_verify_ssl(self, parsed_globals):
self.parameters['verify_ssl'] = parsed_globals.verify_ssl

def add_page_size(self, parsed_globals):
self.parameters['page_size'] = parsed_globals.page_size
def add_page_size(self, parsed_args):
self.parameters['page_size'] = getattr(parsed_args, 'page_size', None)
4 changes: 0 additions & 4 deletions awscli/data/cli.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@
"query": {
"help": "<p>A JMESPath query to use in filtering the response data.</p>"
},
"page-size": {
"type": "int",
"help": "<p>Specifies the page size when paginating.</p>"
},
"profile": {
"help": "<p>Use a specific profile from your credential file.</p>"
},
Expand Down
10 changes: 10 additions & 0 deletions tests/integration/customizations/s3/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,16 @@ def test_rm_with_newlines(self):
# And verify it's gone.
self.assertFalse(self.key_exists(bucket_name, key_name='foo\r.txt'))

def test_rm_with_page_size(self):
bucket_name = self.create_bucket()
self.put_object(bucket_name, 'foo.txt', contents='hello world')
self.put_object(bucket_name, 'bar.txt', contents='hello world2')
p = aws('s3 rm s3://%s/ --recursive --page-size 1' % bucket_name)
self.assert_no_errors(p)

self.assertFalse(self.key_exists(bucket_name, key_name='foo.txt'))
self.assertFalse(self.key_exists(bucket_name, key_name='bar.txt'))


class TestCp(BaseS3CLICommand):

Expand Down
11 changes: 5 additions & 6 deletions tests/unit/customizations/s3/test_subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ def setUp(self):

def test_ls_command_for_bucket(self):
ls_command = ListCommand(self.session)
parsed_args = FakeArgs(paths='s3://mybucket/', dir_op=False)
parsed_args = FakeArgs(paths='s3://mybucket/', dir_op=False, page_size='5')
parsed_globals = mock.Mock()
parsed_globals.page_size = '5'
ls_command._run_main(parsed_args, parsed_globals)
call = self.session.get_service.return_value.get_operation\
.return_value.call
Expand All @@ -79,7 +78,7 @@ def test_ls_command_for_bucket(self):
def test_ls_command_with_no_args(self):
ls_command = ListCommand(self.session)
parsed_global = FakeArgs(region=None, endpoint_url=None, verify_ssl=None)
parsed_args = FakeArgs(dir_op=False, paths='s3://')
parsed_args = FakeArgs(dir_op=False, paths='s3://')
ls_command._run_main(parsed_args, parsed_global)
# We should only be a single call.
self.session.get_service.return_value.get_operation.assert_called_with(
Expand All @@ -93,7 +92,7 @@ def test_ls_command_with_no_args(self):
args = get_endpoint.call_args
self.assertEqual(args, mock.call(region_name=None, endpoint_url=None,
verify=None))

def test_ls_with_verify_argument(self):
options = {'default': 's3://', 'nargs': '?'}
ls_command = ListCommand(self.session)
Expand All @@ -107,7 +106,7 @@ def test_ls_with_verify_argument(self):
self.assertEqual(args, mock.call(region_name='us-west-2',
endpoint_url=None,
verify=False))


class CommandArchitectureTest(S3HandlerBaseTest):
def setUp(self):
Expand All @@ -131,7 +130,7 @@ def tearDown(self):
super(CommandArchitectureTest, self).tearDown()
clean_loc_files(self.loc_files)
s3_cleanup(self.bucket, self.session)

def test_set_endpoint_no_source(self):
cmd_arc = CommandArchitecture(self.session, 'sync',
{'region': 'us-west-1',
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/customizations/test_paginate.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,14 @@ def test_customize_arg_table(self):
# We also need to inject startin-token and max-items.
self.assertIn('starting-token', argument_table)
self.assertIn('max-items', argument_table)
self.assertIn('page-size', argument_table)
# And these should be PageArguments.
self.assertIsInstance(argument_table['starting-token'],
paginate.PageArgument)
self.assertIsInstance(argument_table['max-items'],
paginate.PageArgument)
self.assertIsInstance(argument_table['page-size'],
paginate.PageArgument)

def test_operation_with_no_paginate(self):
# Operations that don't paginate are left alone.
Expand Down
27 changes: 0 additions & 27 deletions tests/unit/test_clidriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -656,33 +656,6 @@ def test_invoke_with_no_credentials(self):
with self.assertRaises(NoCredentialsError):
caller.invoke(None, None, None)

def test_invoke_with_page_size(self):
operation_object = mock.Mock()
paginate = operation_object.paginate
operation_object.can_paginate = True
parsed_globals = mock.Mock()
parsed_globals.paginate = True
parsed_globals.page_size = '10'
parameters = {}
caller = CLIOperationCaller(self.session)
with mock.patch('awscli.clidriver.CLIOperationCaller._display_response'):
caller.invoke(operation_object, parameters, parsed_globals)
self.assertEqual(paginate.call_args[1], {'page_size': u'10'})

def test_invoke_with_no_page_size(self):
operation_object = mock.Mock()
paginate = operation_object.paginate
operation_object.can_paginate = True
parsed_globals = mock.Mock()
parsed_globals.paginate = True
parsed_globals.page_size = None
parameters = {}
caller = CLIOperationCaller(self.session)
with mock.patch('awscli.clidriver.CLIOperationCaller._display_response'):
caller.invoke(operation_object, parameters, parsed_globals)
# No parameters were passed to it (i.e. only self and endpoint).
self.assertEqual(len(paginate.call_args), 2)


class TestVerifyArgument(BaseAWSCommandParamsTest):
def setUp(self):
Expand Down
12 changes: 6 additions & 6 deletions tests/unit/test_completer.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@

LOG = logging.getLogger(__name__)

GLOBALOPTS = ['--debug', '--endpoint-url', '--no-verify-ssl',
'--no-paginate', '--output', '--page-size', '--profile',
'--region', '--version', '--color', '--query', '--no-sign-request']
GLOBALOPTS = ['--debug', '--endpoint-url', '--no-verify-ssl', '--no-paginate',
'--output', '--profile', '--region', '--version', '--color',
'--query', '--no-sign-request']

COMPLETIONS = [
('aws ', -1, set(['autoscaling', 'cloudformation', 'cloudsearch',
Expand Down Expand Up @@ -62,7 +62,7 @@
set(['--filters', '--dry-run', '--no-dry-run', '--endpoint-url',
'--no-verify-ssl', '--no-paginate', '--no-sign-request',
'--output', '--profile', '--starting-token', '--max-items',
'--region', '--version', '--color', '--query', '--page-size'])),
'--page-size', '--region', '--version', '--color', '--query'])),
('aws s3', -1, set(['cp', 'mv', 'rm', 'mb', 'rb', 'ls', 'sync', 'website'])),
('aws s3 m', -1, set(['mv', 'mb'])),
('aws s3 cp -', -1, set(['--no-guess-mime-type', '--dryrun',
Expand All @@ -74,7 +74,7 @@
'--content-disposition', '--source-region',
'--content-encoding', '--content-language',
'--expires', '--grants', '--only-show-errors',
'--expected-size']
'--expected-size', '--page-size']
+ GLOBALOPTS)),
('aws s3 cp --quiet -', -1, set(['--no-guess-mime-type', '--dryrun',
'--recursive', '--content-type',
Expand All @@ -86,7 +86,7 @@
'--exclude', '--include',
'--source-region',
'--grants', '--only-show-errors',
'--expected-size']
'--expected-size', '--page-size']
+ GLOBALOPTS)),
('aws emr ', -1, set(['add-instance-groups', 'add-steps', 'add-tags',
'create-cluster', 'create-default-roles',
Expand Down