-
Notifications
You must be signed in to change notification settings - Fork 383
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
feat: add API REST pagination doc #1990
Conversation
Thank you, this looks great! Could you please sign the CLA (just two clicks), then we can merge it right away! |
Done. Thank you. |
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.
Awesome, thank you for your contribution! This looks pretty good and I have some ideas how to improve it further :)
docs/guides/pagination.mdx
Outdated
|
||
### Default behavior and parameters | ||
|
||
By default, Ory paginates list data with a page size of 250 items per page. However, you can customize this behavior using two key |
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 page size is typically dependent on the API and may also change in the future (ie get smaller or larger). There is usually a max page size
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.
Interesting. I couldn't find a max page size value in both the response headers and body of admin/identities. Should I be looking somewhere else?
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's only documented in the code at the moment. It's here: https://github.com/ory/kratos/blob/23f323274616c7383668ecb3c405416b8a0e30f0/identity/handler.go#L202
Unfortunately, the CI for playwright tests has been failing for some time. I can force merge the PR when it's ready to go :) |
Not sure I understand this check that is making the -- Do not attempt to reverse engineer or make assumptions about the |
@vinckr pointed out it was a prettier thing. Updated |
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.
LGTM.
CI failures are unrelated. And as for the formatting, we have a task running, that ensures all code in all repositories is uniformly formatted. Make sure to run make format
if the "Format" task fails, or every time you finalize a change and commit the result. If you use an editor that supports prettier it should format everything automatically, though.
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 @jonas-jonas |
Related Issue or Design Document
N/A
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
Further comments
N/A