-
Notifications
You must be signed in to change notification settings - Fork 959
Add keyset pagination options #1827
Add keyset pagination options #1827
Conversation
gitlab.go
Outdated
// The "next" link in the Link header includes query parameters for the | ||
// next paginated request, but the relevant parameters vary among API | ||
// endpoints. | ||
NextLinkParameters url.Values |
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.
Minor question: I wonder if this field should be omitted, so that BuildKeysetPaginatedRequestOptionFunc
can just take in the NextLink
and lazily parse it. That might clean up the interface a little so that clients don't need to think about how to distinguish NextLink
and NextLinkParameters
.
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.
I think that's a good idea. It seems like the goal of the library right now is more "provide a clean interface without being too opinionated" rather than "provide as little as possible" or "all batteries included." I'll see if I can streamline that a little better.
Thanks for doing this! This looks like a pretty good approach to me. I just had one minor point for consideration. |
@svanharmelen Would you mind looking at this? We noticed that on GitLab.com there are clients such as |
Thanks, this looks good to me! @svanharmelen Could you review/merge? |
I'll have a look at this one tomorrow or Sunday... Looks like something we can get merged 👍🏻 |
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.
I'll take it... Thanks @pwlandoll 👍🏻
With xanzy/go-gitlab#1827, go-gitlab now supports keyset pagination, which is much more efficient for iterating through many pages of data. This commit switches the GitLab CI jobs API to use keyset pagination.
With xanzy/go-gitlab#1827, go-gitlab now supports keyset pagination, which is much more efficient for iterating through many pages of data. This commit switches the GitLab CI `/api/v4/projects/:id/jobs` API to use keyset pagination. The other pipeline API endpoints need keyset pagination support: https://gitlab.com/gitlab-org/gitlab/-/issues/431632
Great, thanks! I was able to pull the latest version and run code like the included example. |
* build(deps): bump github.com/xanzy/go-gitlab from 0.92.3 to 0.94.0 Bumps [github.com/xanzy/go-gitlab](https://github.com/xanzy/go-gitlab) from 0.92.3 to 0.94.0. - [Changelog](https://github.com/xanzy/go-gitlab/blob/main/releases_test.go) - [Commits](xanzy/go-gitlab@v0.92.3...v0.94.0) --- updated-dependencies: - dependency-name: github.com/xanzy/go-gitlab dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * feat: use keyset pagination for retrieving project CI jobs With xanzy/go-gitlab#1827, go-gitlab now supports keyset pagination, which is much more efficient for iterating through many pages of data. This commit switches the GitLab CI `/api/v4/projects/:id/jobs` API to use keyset pagination. The other pipeline API endpoints need keyset pagination support: https://gitlab.com/gitlab-org/gitlab/-/issues/431632 --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This is one possible implementation of keyset-based pagination, as suggested in #815.
The basic flow of offset-based pagination is that the client requests the first page, and the server returns information about the page in headers. The flow for keyset-based pagination differs in that the information for building a request for the next page is not returned in pieces, but in a pre-formatted URL in the
Link
header. The documentation recommends using that link to make the request, which doesn't seem like it will work cleanly with the existing code here. In particular, the link includes a number of query parameters that differ among endpoints that support keyset-based pagination.So, the changes here provide a way to directly use all of the query parameters returned in the header when building the next request.
ListOptions
now includes fields that are shared among all keyset-based paginated endpoints. Other required parameters for paginated endpoints can be added by means of aRequestOptionFunc
, for which there is a builder function included.An alternative would be to have
ListOptions
enumerate each possible query parameter that could be included in a "next Link" header, and return them in theResponse
struct in the same way as existing header values. This would mean that the end-user code for each endpoint using this pagination might look different, i.e. there would be no single way to do all keyset-based paginated endpoints. But, it would make the code less opaque, and avoid using a "magic black box function" to addRequestOptionFunc
s.This PR is mostly a suggestion/WIP to get the conversation moving about how this package might support keyset-based paginated endpoints, so please let me know how it looks!