-
Notifications
You must be signed in to change notification settings - Fork 2
Add an option to limit results with Elastica #213
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
Conversation
|
Thank for this, @odolbeau. I'm sorry there's been no activity from us here. Please could you add something to the Elastica section in the README explaining this new option? This will then be good to merge, I think. |
28c49da to
b4a5448
Compare
|
PR rebased! 👍 |
sampart
left a comment
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.
Thanks for this. Just one last change.
|
|
||
| *Be careful when paginating a huge set of documents. By default, offset + limit | ||
| can't exceed 10000. For more information, see: | ||
| [#213](https://github.com/whiteoctober/Pagerfanta/pull/213#issue-87631892).* |
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.
Could you add an additional sentence saying something like "You can mitigate this by setting the $maxResults parameter when constructing the ElasticaAdapter"? Thanks
b4a5448 to
cb95185
Compare
|
Done |
sampart
left a comment
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.
Perfect, thanks.
|
I'll merge once tests have passed. |
|
I've created release https://github.com/whiteoctober/Pagerfanta/releases/tag/v2.0.1 for this. Thanks again for your input. |
With recent ES version, we have en exception when size + offset > 10000.
BTW it's possible to override this directly in ES configuration even if it's not recommended.
With this new constructor argument, it's possible to limit the number of results even if the dataset contains more results.
In this case, we won't display pages where size + offset > 10000 and we won't have this error: