Skip to content

Add pagination support to egress list#851

Open
biglittlebigben wants to merge 6 commits into
mainfrom
benjamin/egress_pagination
Open

Add pagination support to egress list#851
biglittlebigben wants to merge 6 commits into
mainfrom
benjamin/egress_pagination

Conversation

@biglittlebigben
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread cmd/lk/egress.go Outdated

resItems := res.Items
if remaining := limit - len(items); limit > 0 && len(resItems) > remaining {
resItems = resItems[:remaining]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

just to double check since pages get older as we paginate, if the limit cuts mid-page - should we return its tail rather than the head?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I forgot if we order asc or desc within a page - checking it

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ingress does newest last within a page - I guess that will apply to egress as well so this might be valid.

@milos-lk
Copy link
Copy Markdown

nit: The pagination tests cover the RPC control flow well — request count, page tokens, and when pagination stops. They don’t yet assert the user-visible behavior of --limit: which egress IDs are returned and in what order. (might be related to my other comment above)

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