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

Fall back to deprecated starting token parser #849

Merged
merged 2 commits into from
Mar 28, 2016

Conversation

JordonPhillips
Copy link
Contributor

A recent change to paginators broke customers who were manually crafting pagination tokens in services where they did not make them opaque. This adds support for the old starting token format while still returning next tokens in the new format. If we are unable to parse the starting token in the new format, we'll attempt to parse it with the old format.

This addresses the root cause of aws/aws-cli#1835, but the docs should be updated to reflect the new tokens.

cc @kyleknap @jamesls

A recent change to paginators broke customers who were manually crafting
pagination tokens in services where they did not make them opaque. This
adds support for the old starting token format while still returning
next tokens in the new format.
@JordonPhillips JordonPhillips added the pr/needs-review This PR needs a review from a member of the team. label Mar 23, 2016
This handles parsing of old style starting tokens, and attempts to
coerce them into the new style.
"""
log.debug("Attempting to fall back to old starting token parser.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good to say what the token was that was received that started this attempt.

@kyleknap
Copy link
Contributor

Looks good. You pretty much copied code from the old implementation 🚢. Did you test your implementation with some of the services where we saw regressions? It may not hurt to add an integration or functional test for one of those services like rds or route53.

@jamesls
Copy link
Member

jamesls commented Mar 25, 2016

:shipit: You can also use s3 as well for an integration test. There'a already some existing tests for pagination so it might be easier to build on those tests.

@JordonPhillips
Copy link
Contributor Author

tests added

@kyleknap
Copy link
Contributor

Thanks! 🚢

@JordonPhillips JordonPhillips merged commit 2c2aae5 into boto:develop Mar 28, 2016
@JordonPhillips JordonPhillips deleted the add-deprecated-paginator branch March 28, 2016 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/needs-review This PR needs a review from a member of the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants