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

Add refreshResults & Paginator #53

Merged
merged 4 commits into from
Jun 1, 2024
Merged

Add refreshResults & Paginator #53

merged 4 commits into from
Jun 1, 2024

Conversation

bh2smith
Copy link
Collaborator

@bh2smith bh2smith commented May 28, 2024

We rename a previously internal method _runInner to refreshResults which executes and loops until execution is complete (but does not fetch results). Additionally, we Introduce a new class Paginator which fetches and caches and index result sets by page. This class exposes an api similar to the Dune interface result page navigation:

  • nextPage, previousPage, lastPage and pageN.

Paginator can be constructed from the return value of refreshResults and a client of type ExecutionAPI.

Will want to add docs on how to use this, but the test introduced should give a pretty good idea of how to test it out in a front end.

Note that

  1. the first commit (00ac359) is unrelated (fixing e2e test based on Dune's internal data format changes and expired query results).

  2. the second commit (774b789) is just renaming an existing function and making it public (in preparation for public use)

  3. 🔑 the third commit (69c3088) This is the most relevant and substantial change introduced here: Paginator 📃

cc @agaperste.

@agaperste
Copy link

@bh2smith Moving our chat here
Checked in with a few folks who use Dune API, and some front end devs, seems like more folks prefer the current way in SDK which is to separate out execution from Paginator :)

@bh2smith
Copy link
Collaborator Author

folks prefer the current way in SDK which is to separate out execution from Paginator :)

Sounds great @agaperste, so shall we merge and publish this as is?

@agaperste
Copy link

I vote yes :)

@bh2smith
Copy link
Collaborator Author

Would you mind clicking "approve" on the pull request?

@bh2smith bh2smith merged commit a97351b into main Jun 1, 2024
3 checks passed
@bh2smith bh2smith deleted the paginated-refresh branch June 1, 2024 05:39
@agaperste
Copy link

ah sorry just seeing this.. was traveling. but yay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants