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

[Feature] Pagination helper #798

Open
Neztore opened this issue Apr 24, 2024 · 2 comments
Open

[Feature] Pagination helper #798

Neztore opened this issue Apr 24, 2024 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested

Comments

@Neztore
Copy link
Member

Neztore commented Apr 24, 2024

I have included some thoughts here of how it could be implemented, but it is very much open to discussion.

  • It could implement the async iterator protocol where each call to obj.next() will return a promise and the value (i.e. for a logs iterator it would return a specific log - not a page of logs)
  • Include a getAll method that iterates until all pages have been fetched and returns them as a single array (?) or exposes them through the normal iteration system
  • If we implement a standard way of doing this it would allow users to loop over results with ease while hiding the underlying complexity of fetching pages
  • (But that may encourage bad patterns? People looping over all results not realising it is making many requests?)
  • This would be a breaking change if we drop it in. Could we make it default to returning an array and allow a special parameter to return iterators? Or we could make it part of the v5 and replace it - but it is an increase in complexity to understand these iterators.
@Neztore Neztore added enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels Apr 24, 2024
@alanbixby
Copy link
Member

TODO: Per working group discussion, @Regalijan - investigate if the existing event listener methods (onAuditLog.js etc) are still trying to fetch all of the pages instead of short polling against only the latest page due to no limit being passed to the pagination function

@Regalijan
Copy link
Member

As far as I can tell, no. Most of the event methods don't use the paginator function to start with. Non-integers result in a limit of 100 for the ones that do. Network logs are only showing the first page being fetched either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants