Skip to content

Properly handle pagination in the REST backend #39

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ukleinek
Copy link
Contributor

While working on integrating patchwork in my mail setup I noticed that pwclient list only lists up to 30 patches. So the motivation for this patch series is commit 7477b4d. The other changes are preparing this or simple cleanups noticed on the way.

ukleinek added 2 commits May 22, 2025 17:55
As a desired side effect this allows to let api.patch_list() return a
generator instead of a list.
ukleinek added 3 commits May 22, 2025 18:49
If there are many patches (or other objects) to be listed, patchwork
only returns the first 30 by default. In such a case continue fetching
objects using the "next" link provided in the Link: header.

Note this changes REST._list() to return a generator instead of a list.
All callers only use it to generate a list, so this isn't a problem.
Similar to commit 70f96f9 ("support the delegate filter for REST
API") which added the delegate filter this implements an exact match
only and not (as promised by `pwclient list -h`) as substring match.

But for the same reason as for the delegate filter this is better than
ignoring a passed submitter.
This has the advantage that with pagination the first entries are already
provided before later entries are queried from the server. Also if the
generator isn't walked through completely, only the needed queries are
actually executed.

All callers of these functions can cope with the returned object being a
generator now instead of a list as they just iterate once over it.
@jacob-keller
Copy link
Contributor

I like your cleanups and would prefer we take this over my PR. I do think that if we are going to depend on requests it may make sense to use it for more than just the one function... But that could always be a future cleanup.

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