Add support for page_size parameter in CursorPaginator class by cypreess · Pull Request #5250 · encode/django-rest-framework (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation4 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

cypreess

CursorPaginator receives identical support for page size query parameter as PageNumberPagination.
Solving #5249

@cypreess

@tomchristie

Hiya! The code looks good, but I'm still in two minds about if we should accept extra behavior on the already-complex cursor pagination. Not sure how to best take a judgement on this right now.

@cypreess

@tomchristie I actually don't understand where is that complexity you mention. In pagination you have two concepts:

  1. logic for finding page starting position - this can be anything, an offset, a cursor to value used for ordering, etc.
  2. logic for saying how many elements to retrieve from start position - here just a single integer param of page_size

The only doubt I have here is about the role of offset_cutoff in whole story.

@joshmaker

I hope this can be merged in at some point, because we really like this feature at The Atlantic. Ignoring the extensive unit tests, it's only a little bit more code and makes the CursorPaginator API work more like the other built in paginators.

rpkilby

@carltongibson

carltongibson pushed a commit that referenced this pull request

Oct 6, 2017

Moved issue links to top for easier access. (Can move back later)

Strict JSON handling

Remove Django 1.8 & 1.9 from README and setup.py

Tickets migrated to 3.7.0 milestone.

Move issue links back to bottom.