-
-
Notifications
You must be signed in to change notification settings - Fork 934
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
Limit number of requests in pagination #1181
Conversation
readme.md
Outdated
###### pagination.requestLimit | ||
|
||
Type: `number`\ | ||
Default: `100` |
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.
Why 100
? If this is just meant to prevent a mistake in development, I think the default should be higher. Maybe just Infinity
. Then you just set it if you need it.
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.
Okay, I increased it to 10000. Defaulting to Infinity
is too much I think as also at runtime in production you could trigger accidentally an infinite amount.
readme.md
Outdated
Type: `number`\ | ||
Default: `100` | ||
|
||
The maximum amount of request that should be triggered. |
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.
The text should include the intended use-case for this option.
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 just updated the paragraph with an example.
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.
It should be noted that retry requests does not count.
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.
Just added that note as well.
readme.md
Outdated
|
||
The maximum amount of request that should be triggered. [Retries on failure](#retry) are not count towards this limit. | ||
|
||
For example, if you need the first 10 pages but don't know how many items there are per page. It can also be helpful during development to avoid an infinite number of requests. |
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.
For example, if you need the first 10 pages but don't know how many items there are per page.
It's not clear how setting a request limit helps with this? It seems like setting a hard-coded limit goes against not knowing how many items there are.
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.
are not count
-> are not counted
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.
@szmarczak it's fixed.
Currently, it's easily possible to trigger an infinite amount of requests during development because we use a
while(true)
construct. This PR adds a new option to limit the number of requests.Checklist