-
Notifications
You must be signed in to change notification settings - Fork 61
fix(rest-api-client): set cursor size to 500 in getAllRecordsWithCursor #3537
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR optimizes the getAllRecordsWithCursor method by setting the cursor size to the maximum allowed value of 500 records per request (up from the default of 100). This change significantly reduces the number of API calls needed to fetch large record sets, improving performance by up to 5x for operations that retrieve many records.
Key changes:
- Modified
getAllRecordsWithCursorto explicitly setsize: 500when creating cursors - Updated the corresponding test to verify the size parameter is correctly passed
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/rest-api-client/src/client/RecordClient.ts |
Added size: 500 parameter when calling createCursor within getAllRecordsWithCursor method |
packages/rest-api-client/src/client/__tests__/record/AllRecords.test.ts |
Updated test to verify cursor is created with size 500 parameter |
I've completed my review of this pull request. The implementation is correct, well-tested, and follows the existing codebase patterns. The change is a straightforward performance optimization that:
-
Correctly uses the maximum allowed cursor size: The Kintone API documentation confirms that 500 is the maximum allowed size for cursors (as documented in
packages/rest-api-client/docs/record.mdline 488). -
Is backward compatible: The change is purely internal - it doesn't affect the public API surface or behavior from the user's perspective.
-
Has appropriate test coverage: The test was correctly updated to verify the new behavior.
-
Benefits multiple methods: Since
getAllRecordswithwithCursor=trueinternally callsgetAllRecordsWithCursor, it also benefits from this optimization.
The code changes are clean and minimal. No issues were found during the review.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { id } = await this.createCursor(params); | ||
| const paramsWithSize = { | ||
| ...params, | ||
| size: 500, |
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.
| const DELETE_RECORDS_LIMIT = 100; |
を使う方が良さそう
| size: 500, | |
| size: GET_RECORDS_LIMIT, |
Why
refs: kintone/js-sdk-ja#51
The
getAllRecordsWithCursormethod internally usescreateCursorAPI, which has a defaultsizeof 100 records per request. However, the maximum allowed value is 500. By using the maximum size, we can reduce the number of API calls needed to fetch all records, improving performance.For example, fetching 1000 records:
What
size: 500when callingcreateCursoringetAllRecordsWithCursormethodHow to test
Checklist
pnpm lintandpnpm teston the root directory.