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

flowctl: Update pagination to use 0-indexed inclusive ranges, and page by response length rather than fixed page size. #1637

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

jshearer
Copy link
Contributor

@jshearer jshearer commented Sep 18, 2024

This change is Reviewable

@jshearer jshearer force-pushed the jshearer/pagination_boundary_fix branch from 1ebba9c to 9d83b1e Compare September 18, 2024 15:55
@jshearer jshearer added the change:unplanned This change is unplanned, useful for things like docs label Sep 18, 2024
@jshearer jshearer marked this pull request as ready for review September 18, 2024 15:58
@jshearer jshearer force-pushed the jshearer/pagination_boundary_fix branch from 9d83b1e to 60fa17e Compare September 18, 2024 15:59
@jshearer jshearer changed the title flowctl: Keep track of next page offset considering that responses may sometimes include more than page_size rows flowctl: Update pagination to use 0-indexed inclusive ranges, and page by response length rather than fixed page size. Sep 18, 2024
Copy link
Member

@psFried psFried left a comment

Choose a reason for hiding this comment

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

LGTM

crates/flowctl/src/pagination.rs Show resolved Hide resolved
…y sometimes include more than `page_size` rows
@jshearer jshearer force-pushed the jshearer/pagination_boundary_fix branch from 60fa17e to bc73e19 Compare September 18, 2024 16:14
@jshearer jshearer merged commit 2e904fb into master Sep 18, 2024
3 checks passed
@jgraettinger jgraettinger deleted the jshearer/pagination_boundary_fix branch September 18, 2024 17:04
github-actions bot pushed a commit to estuary/homebrew-flowctl that referenced this pull request Sep 19, 2024
## What's Changed
* Batch draft spec upserts, rather than attempting to paginate the response by @jshearer in estuary/flow#1629
* use max build_id for validating local specs by @psFried in estuary/flow#1636
* Update pagination to use 0-indexed inclusive ranges, and page by response length rather than fixed page size. by @jshearer in estuary/flow#1637
github-actions bot pushed a commit to estuary/homebrew-flowctl that referenced this pull request Sep 19, 2024
## What's Changed
* Batch draft spec upserts, rather than attempting to paginate the response by @jshearer in estuary/flow#1629
* use max build_id for validating local specs by @psFried in estuary/flow#1636
* Update pagination to use 0-indexed inclusive ranges, and page by response length rather than fixed page size. by @jshearer in estuary/flow#1637
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:unplanned This change is unplanned, useful for things like docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants