-
Notifications
You must be signed in to change notification settings - Fork 117
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 pagination to list APIs. #422
Comments
We can use a Cursor with this data
When the client will call the gateway API it will can provide some query values like limit, orderby, asc and some filters(if expected), for example: There are some agola cmd list. We should also add here the cursor asking him in the args. Do you can confirm? The APIs will return the data and pagination for example:
where PaginationInfo contains:
The client will use for the next calls only NextCursor for ask the next page or PrevCursor for ask the previous page, if the values are present(for example the first page don't have PrevCursor). The Gateway take the query values(limit, orderby,ecc.), or the cursor(that contains all the data needed), and use them to call the configstore/runservice. The decode and management of the cursor is done by the configstore/runservice that will get the data and create the pagination(PaginationInfo) with the cursors. There are some APIs where it is not possible to implement cursor pagination, as secrets and variables, because they do tree querys when the client use tree parameter. |
I'm not sure I understand this.
We could also keep start if useful for some api to fetch from a specific id with specific option. Then for the next results the cursor could be used.
The output should contain a cursor. But there will only be one cursor. It doesn't makes sense to have a prev or next cursor. It'll just be a cursor that point to the next batch of data based on the original query. If the original query has a sort order the cursor will use that.
It's the gateway that manages the exposed API and so it MUST also manage the cursor, since it could call multiple underlying services to create a response. The underlying services don't require a cursor since they aren't exposed.
I don't see why this won't work. Can you explain it? Remember that if you're going to implement this, also agola-web should be changed and the related PRs should be created to be able to test them. |
I remove PointsNext since we maintain only one cursor for the next page, so the cursor contains this data:
The response of the gateway can be, for example:
or we can create a generic response like this:
What do you like is better? We keep start for some api where is already implemented. @sgotti we should add the cursor to the command line list? And the command result must show the cursor to the user. |
Don't be rigid on this. Filters, but also the other fields could be different based on the api.
The latter doesn't make sense.
By default the client could list all the entries by fetching all of them doing multiple calls to the api. |
|
There's no need to make cursor the same type for every api. |
@sgotti when the user call the last page do you think is better to return an empty cursor(so it know there are no other pages to ask) or the cursor with the last data id(the same as the other calls)?
|
I'll return an empty cursor.
The underlying services could return if there's more data themself. |
@sgotti with the APIs list secrets/variables I think the pagination can not work when the client use the
If the client use parameter limit=2 the Gateway should return I suggest to avoid/ignore limit when the client use removeoverridden parameter, or avoid limit for this APIs(secrets/variables). |
@alessandro-sorint I'm not sure I understand. If the api returns reproducible output as it should, then the cursor will contain the information on the next batch of data. How you get them is an implementation detail.
I don't think Limit should be part of the cursor but should be set every time by the call and on the server side there'll be an hard limit. |
Current "list" APIs (with few exceptions) return all the results. This was meant to be only temporary since a sort of pagination is always needed to avoid returning too much data.
There are different options:
I'll prefer cursor based pagination (like already done for user/project get runs APIs).
NOTES
This will break current API but the APIs are not yet stable.
Additional required Actions
The text was updated successfully, but these errors were encountered: