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

Make GetLogEvents pagination API consistent with others #2419

Open
frosforever opened this issue Jun 16, 2021 · 3 comments
Open

Make GetLogEvents pagination API consistent with others #2419

frosforever opened this issue Jun 16, 2021 · 3 comments
Assignees
Labels
feature-request This issue requests a feature. p3 This is a minor priority issue paginator

Comments

@frosforever
Copy link

To paginate responses from GetLogEvents api currently requires either nextForwardToken or nextBackwardToken depending on is startFromHead is true. This means it does not seem to neatly fit into the pagination model and adding

"GetLogEvents": {
      "input_token": "nextForwardToken",
      "output_token": "nextToken",
      "limit_key": "limit",
      "result_key": "events"
}      

to https://github.com/frosforever/botocore/blob/develop/botocore/data/logs/2014-03-28/paginators-1.json would not work to automatically paginate results.

Describe the solution you'd like
Ideally, API would be return a single nextToken in the response that would either go forward or backward based on the original starting request.

Thanks very much for reviewing my issue. I'm not sure if this belongs in this repo or if this something that's controlled by the internal AWS Cloudwatch logs team. Feel free to close if this doesn't belong here.

@frosforever frosforever added feature-request This issue requests a feature. needs-triage This issue or PR still needs to be triaged. labels Jun 16, 2021
@kdaily kdaily self-assigned this Jun 17, 2021
@kdaily kdaily removed the needs-triage This issue or PR still needs to be triaged. label Jun 17, 2021
@kdaily
Copy link
Member

kdaily commented Jun 17, 2021

Hi @frosforever,

Thanks for your comment. You assume correctly - the internal AWS Cloudwatch team controls the naming of the parameters here. But it looks like here you're just requesting that this operation be paginated!

The name of that parameter is an implementation detail, as the AWS CLI, Python SDK (boto3) and other AWS SDKs abstract over this and provide paginator functionality:

https://boto3.amazonaws.com/v1/documentation/api/latest/guide/paginators.html

I can certainly request that a paginator be added for this operation by the AWS Cloudwatch team - paginator models are used by other AWS SDKs so are added upstream.

@kdaily kdaily added paginator response-requested Waiting on additional info and feedback. labels Jun 17, 2021
@frosforever
Copy link
Author

@kdaily Yes pagination is the goal!

Perhaps my issue is based on a misunderstanding of how the paginator works. The complexity with GetLogEvents is not so much the name of the next token but more that a different field is used as the next token depending on if you're going forward or backward in time.

From https://docs.aws.amazon.com/AmazonCloudWatchLogs/latest/APIReference/API_GetLogEvents.html#API_GetLogEvents_RequestSyntax:

startFromHead

If the value is true, the earliest log events are returned first. If the value is false, the latest log events are returned first. The default value is false.
If you are using a previous nextForwardToken value as the nextToken in this operation, you must specify true for startFromHead.

If there's a way to support pagination even with that complexity I am all for it! Thanks for taking the time to triage and notify upstream.

@tim-finnigan
Copy link
Contributor

Linking related issue: #1474

@RyanFitzSimmonsAK RyanFitzSimmonsAK added the p3 This is a minor priority issue label Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request This issue requests a feature. p3 This is a minor priority issue paginator
Projects
None yet
Development

No branches or pull requests

4 participants