Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Prefetch data while running Bubbletea pagination #476

Closed
wants to merge 10 commits into from

Conversation

zychen5186
Copy link
Contributor

@zychen5186 zychen5186 commented Apr 27, 2024

TL;DR

The get function takes more time to fetch data from the remote server as the amount increases, hence we only fetch 100 rows initially, and prefetch 100 rows of data at once while the user scrolls through pages to enhance the user experience.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

For testing run:

make compile
./bin/flytectl get execution -p my-project -d development -i

We initially fetch the first 100 rows when the paginator is called and start to prefetch the next batch (next 100 rows).
Prefetch will be triggered when the current page exceeds a prefetch threshold.
The first or last batch will be discarded when it exceeds the cache limit (currently set to 1000 rows).
We added a loading animation to indicate the fetching process is ongoing.
Apr-27-2024 17-32-22

Users can use --filter.page flag to specify their starting page, if not specified, page will start from 1.
Apr-28-2024 16-21-42

Tracking Issue

flyteorg/flyte#4440

Follow-up issue

Follow-up of #473

Big thanks to @troychiu for discussing and debugging with me!! 馃檶 鉂わ笍

troychiu
troychiu previously approved these changes Apr 29, 2024
Copy link
Member

@troychiu troychiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome

pkg/bubbletea/bubbletea_pagination_util.go Show resolved Hide resolved
@eapolinario
Copy link
Contributor

Can you merge master to get rid of the failure in the unit tests?

@zychen5186
Copy link
Contributor Author

Can you merge master to get rid of the failure in the unit tests?

Sure!

Signed-off-by: zychen5186 <brianchen5197@gmail.com>
Signed-off-by: zychen5186 <brianchen5197@gmail.com>
Signed-off-by: zychen5186 <brianchen5197@gmail.com>
Signed-off-by: zychen5186 <brianchen5197@gmail.com>
Signed-off-by: zychen5186 <brianchen5197@gmail.com>
Signed-off-by: zychen5186 <brianchen5197@gmail.com>
Signed-off-by: zychen5186 <brianchen5197@gmail.com>
Signed-off-by: zychen5186 <brianchen5197@gmail.com>
Signed-off-by: zychen5186 <brianchen5197@gmail.com>
Signed-off-by: zychen5186 <brianchen5197@gmail.com>
@zychen5186 zychen5186 marked this pull request as ready for review April 30, 2024 18:53
@troychiu
Copy link
Member

troychiu commented May 1, 2024

close because of moving to monorepo

@troychiu troychiu closed this May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants