-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
docs: document server side pagination proposal #17222
docs: document server side pagination proposal #17222
Conversation
Signed-off-by: Alexander Matyushentsev <[email protected]>
Looking forward for this 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be a great feature and a no-brainer. Do you need additional reviewers/approvers? I'd be happy to volunteer if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for putting this together!
I do think that it's the right thing to move things like filtering from the client side to the server side.
**Pagination Cursor** | ||
|
||
It is proposed to add `offset` and `limit` fields for pagination support in Application List API. | ||
The The Watch API is a bit more complex. Both Argo CD user interface and CLI are relying on the Watch API to display real time updates of Argo CD applications. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The The Watch API is a bit more complex. Both Argo CD user interface and CLI are relying on the Watch API to display real time updates of Argo CD applications. | |
The Watch API is a bit more complex. Both Argo CD user interface and CLI are relying on the Watch API to display real time updates of Argo CD applications. |
I am not sure if I am missing something, but can we merge this? |
This looks great; thanks for writing this up. |
@crenshaw-dev So Alex has been out sick the past few days. Sorry for the delay but I'll have him follow as soon as he's back |
@christianh814 It's been a while since your last comment. I hope @alexmt has recovered. Maybe one of you two can answer the following questions:
|
I will let @alexmt update on this.
Yes we have an implementation on this already. Alex has needs to make some changes to generalize it. He posted about it HERE |
so are we ready to ship it? 🎆 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Enough information in this proposal to start the implementation. Some improvements could be
- Extract the fields for query filtering in their own objects such as ApplicationQueryFilter
- Research the pagination best practice and evaluate if offset/limit is the best strategy
- Describe the metadata that will be returned to the UI (such as total items, etc.)
Signed-off-by: Alexander Matyushentsev <[email protected]> Signed-off-by: alnoor <[email protected]>
Signed-off-by: Alexander Matyushentsev <[email protected]> Signed-off-by: austin5219 <[email protected]>
Signed-off-by: Alexander Matyushentsev <[email protected]> Signed-off-by: Adrian Aneci <[email protected]>
PR contains a proposal for a server-side pagination feature. #14947