brendanhay / amazonka

A comprehensive Amazon Web Services SDK for Haskell.
https://amazonka.brendanhay.nz
Other
605 stars 227 forks source link

amazonka-cloudwatch-logs: GetLogEvents is not paginated #356

Open phyrex1an opened 7 years ago

phyrex1an commented 7 years ago

The GetLogEvents (https://docs.aws.amazon.com/AmazonCloudWatchLogs/latest/APIReference/API_GetLogEvents.html) API call can take nextToken as usual, but the response is a bit unusual in that it returns both a nextBackwardToken and nextForwardToken. I think that's what confusing the code generator.

I think correct behavior is following nextForwardToken until nextForwardToken stops changing in between invocations. Doing this does not seem to be possible with the current AWSPager class so maybe fetching until events is an empty list can work as well, though I'm not certain if that is correct in all cases.

Something like

  instance AWSPager GetLogEvents where
    page rq rs
               | not (isJust (rs ^. glersNextForwardToken)) = Nothing
               | not (null (rs ^. glersEvents)) = Nothing
               | otherwise = Just $ rq & gleNextToken .~ rs ^. glersNextForwardToken
endgame commented 3 years ago

If the Go SDK is to be believed, iterating until an empty response is not safe:

This operation can return empty results while there are more log events available through the token.

It also appears that the awkwardness of the API means it's not paginated in botocore either: https://github.com/boto/botocore/issues/2419

At this point I was going to give up and close the issue but the Java client does have a forward paginator defined:

https://github.com/aws/aws-sdk-java-v2/blob/d98b79d78cf17335d6f1353aaec9e1d757b2def4/services/cloudwatchlogs/src/main/resources/codegen-resources/paginators-1.json

I'll put that paginator definition into a PR and see what the generated code looks like.

endgame commented 3 years ago

That paginator does not generate good code for amazonka: it tries to stop if nextForwardToken is Nothing, but the API will return the token it was given if there's nothing new. It will also stop if events is Nothing, but we are told to expect empty events even if we're not at the end of the paging. It will request another page otherwise, which means that it will try to page forever. This isn't a trivial fix, so it's a post-2.0 problem.

endgame commented 3 years ago

Given that it's not in botocore and I suspect the java pager will likely try to page indefinitely also, I think there's an argument for just closing this issue. But I'll leave it open in case someone want to try refactoring how paginators are parsed/generated.