Update sync to use pagination #5
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the sync script to use pagination when fetching data from the Kimai API, replacing the previous approach of requesting all records in a single API call with size=PHP_INT_MAX.
Key changes:
- Introduces a pagination loop that fetches data in pages of 500 records
- Removes the hardcoded
size=PHP_INT_MAXparameter from the timesheets endpoint - Accumulates results across multiple API calls before processing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if ($results === false) { | ||
| $io->error(sprintf('Failed to sync data for endpoint: %s', $settings['endpoint'])); | ||
| break; |
There was a problem hiding this comment.
When an API error occurs mid-pagination, the function breaks out of the loop and continues processing with partial data from previous pages. This could lead to incomplete or inconsistent data in the database. Consider returning early or throwing an exception instead of breaking, to prevent processing partial results.
| break; | |
| return; |
| $page = 1; | ||
| while (true) { | ||
| $separator = (strpos($settings['endpoint'], '?') === false) ? '?' : '&'; | ||
| $url = sprintf('%s%spage=%s&size=500', $settings['endpoint'], $separator, $page); |
There was a problem hiding this comment.
The page size is hardcoded to 500. Consider making this configurable through a constant or configuration parameter to allow flexibility for different environments or API rate limits.
| while (true) { | ||
| $separator = (strpos($settings['endpoint'], '?') === false) ? '?' : '&'; | ||
| $url = sprintf('%s%spage=%s&size=500', $settings['endpoint'], $separator, $page); | ||
| $results = $doGet($client, $url); |
There was a problem hiding this comment.
The $doGet function doesn't handle Guzzle HTTP exceptions. If an HTTP error (like 500, 401, 403) or network error occurs during pagination, it will cause an uncaught exception that crashes the entire sync process. This is particularly problematic in a pagination loop where partial data may have already been collected. Consider wrapping the API call in a try-catch block to handle errors gracefully, similar to how it's done at line 312 for the teams sync.
1 participant
No description provided.