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 infinite loop on cloudwatchlogs GetLogEventsPages #1908

Closed
wants to merge 1 commit into from

Conversation

johanneswuerbach
Copy link

@johanneswuerbach johanneswuerbach commented Apr 21, 2018

Cloudwatch log event responses always include the next token even when there are no more results and pagination will never terminate.

More details aws/aws-sdk-ruby#712

This replicates a similar fix applied to the ruby aws-sdk aws/aws-sdk-ruby#730

Cloudwatch log event responses always include the next token and pagination
will never terminate.
@jasdel jasdel added Review Needed service-api This issue is due to a problem in a service API, not the SDK implementation. labels Apr 23, 2018
@jasdel
Copy link
Contributor

jasdel commented Apr 23, 2018

Thanks for creating this PR @johanneswuerbach, was this a recent issue that started to fail? I noticed the Ruby issue is back in 2015 so i'm curious if this is a new regression with CloudWatchLogs, or a bug that has always existed.

Is there a specific condition where this error occurs with CloudWatchLogs, does this error occur for any pagination of the CloudWatchLogs GetLogEvents API?

@jasdel jasdel added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 23, 2018
@johanneswuerbach
Copy link
Author

I just started to use this API and noticed the never ending pagination with any kind of provided input.

The current ruby sdk also still has the last page condition changes added in 2015, so I would say those changes are still valid https://github.com/aws/aws-sdk-ruby/blob/4edc8bc84b8994b9db497ecd5ad229348252bb37/gems/aws-sdk-core/lib/aws-sdk-core/pager.rb#L42-L44

@johanneswuerbach
Copy link
Author

johanneswuerbach commented May 22, 2018

Any additional details I can provide @jasdel?

An example requiring a workaround can now be seen here https://github.com/virtual-kubelet/virtual-kubelet/blob/b4cb8099681118f8049234f12328f7a73c4b1a50/providers/aws/fargate/cluster.go#L330-L344

//cc @ofiliz

@xibz
Copy link
Contributor

xibz commented May 22, 2018

Hello @johanneswuerbach, I've went ahead and expanding on your PR to allow for users to opt into this functionality in addition to fixing the paginator for the service. Please see #1945! And thank you again for taking the time to do this. I'll go ahead and close this PR.

@xibz xibz closed this May 22, 2018
@awstools awstools mentioned this pull request Jun 25, 2018
@diehlaws diehlaws added needs-review This issue or pull request needs review from a core team member. and removed review-needed labels Jan 4, 2019
@diehlaws diehlaws removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review This issue or pull request needs review from a core team member. service-api This issue is due to a problem in a service API, not the SDK implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants