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

Enabled --page-size as global argument. #889

Merged
merged 1 commit into from
Aug 21, 2014
Merged

Conversation

kyleknap
Copy link
Contributor

The page size controls the amount of items returned by
each page while paginating. Smoke tests were added for ec2
and s3api to ensure the parameter does what is expected to
do for known paginating operations.

cc @jamesls @danielgtaylor

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling 7bc2e3a on kyleknap:page-size into c560dfe on aws:develop.

@@ -546,6 +546,8 @@ 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 getattr(parsed_globals, 'page_size', None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is getattr() needed here? If it's a global arg you should just be able to access parsed_globals.page_size directly right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the unit test failed via a key error when I did not use getattr(). I am going to change it and see if the unit tests need to be updated or something else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the test just needed to be updated.

@jamesls
Copy link
Member

jamesls commented Aug 21, 2014

Overall looks good, just a few questions above.

The page size controls the amount of items returned by
each page while paginating. Smoke tests were added for ``ec2``
and ``s3api`` to ensure the parameter does what is expected to
do for known paginating operations.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) when pulling 6bf5851 on kyleknap:page-size into c560dfe on aws:develop.

@kyleknap
Copy link
Contributor Author

Changes based on comments are in the latest commit

@jamesls
Copy link
Member

jamesls commented Aug 21, 2014

:shipit: Looks good.

kyleknap added a commit that referenced this pull request Aug 21, 2014
Enabled ``--page-size`` as global argument.
@kyleknap kyleknap merged commit 1db0a08 into aws:develop Aug 21, 2014
@kyleknap kyleknap deleted the page-size branch August 21, 2014 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants