Skip to content
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

Re-add ability to customize paginator class #3684

Closed
jezdez opened this issue Nov 30, 2015 · 4 comments
Closed

Re-add ability to customize paginator class #3684

jezdez opened this issue Nov 30, 2015 · 4 comments

Comments

@jezdez
Copy link

jezdez commented Nov 30, 2015

As part of the 3.1 release DRF lost the ability to override the paginator class and replaced it with a custom implementation that uses Django's default paginator in the critical paginate_queryset method. This change severely limits the architectural separation of data pagination from view level pagination behavior.

E.g. code that happens when the pagination page queryset is evaluated now needs to be handled in an overridden Pagination.paginate_queryset method. Since the instantiation of the paginator is in the middle of the default PageNumberPagination.paginate_queryset method working around this deprecation results in effectively completely rewriting it and making it a future tech debt concern. The alternative to run queryset evaluation time code in paginate_queryset and then call the parent paginate_queryset is equally not satisfactory as at that point the page object hasn't been created yet which contains vital information that is not available on the pagination instance. Fetching that data after the fact would again require evaluation the queryset, again not wanted.

As such the removal of the ability to override the paginator class is a step backwards that leads to brittle workarounds (monkeypatches) and removed the ability to hook into a key moment of pagination. Please advise what I can do to improve this.

@tomchristie tomchristie added this to the 3.3.2 Release milestone Nov 30, 2015
@tomchristie
Copy link
Member

Is the crux of this issue that you'd like to be able to override the behavior of this line without having to override the rest of the method?

If so, does this PR address your concerns?... #3631

@jezdez
Copy link
Author

jezdez commented Nov 30, 2015

Yep!

@jezdez
Copy link
Author

jezdez commented Nov 30, 2015

(to both questions)

@jezdez
Copy link
Author

jezdez commented Nov 30, 2015

Closing as duplicate of #3631

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants