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

Fix issue with shadowed max-items arguments #729

Merged
merged 2 commits into from
Mar 31, 2014
Merged

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Mar 31, 2014

When we add pagination configs for a service, we hide
the original manual pagination arguments in favor of the normalized
arguments. However, to support backwards compatability we still
allow users to specify the manual arguments. If we detect this is the
case we will turn off pagination.

However, when the normalized argument matches the manual paging argument
(as is the case when a service users MaxItems), we have an ambiguous
case. Did they mean the back-compat version of the normalized argument
version? Previously we were assuming the user meant the manual paging
argument. Now we assume that they meant the normalized paging
arguments.

Services impacted:

  • route53
  • iam
  • ses

When we add pagination configs for a service, we hide
the original manual pagination arguments in favor of the normalized
arguments.  However, to support backwards compatability we still
allow users to specify the manual arguments.  If we detect this is the
case we will turn off pagination.

However, when the normalized argument matches the manual paging argument
(as is the case when a service users `MaxItems`), we have an ambiguous
case.  Did they mean the back-compat version of the normalized argument
version?  Previously we were assuming the user meant the manual paging
argument.  Now we assume that they meant the normalized paging
arguments.

Services impacted:

* route53
* iam
* ses
We don't pass max-items anymore.
@jamesls
Copy link
Member Author

jamesls commented Mar 31, 2014

I've fixed the failing test . @danielgtaylor mind taking a look?

@danielgtaylor
Copy link
Contributor

LGTM 🚢-it!

@jamesls jamesls merged commit 6c58adf into aws:develop Mar 31, 2014
@jamesls jamesls deleted the page-args branch June 23, 2014 18:29
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.

None yet

2 participants