Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness and efficiency of API interactions, particularly for file and result uploads. By introducing an intelligent retry mechanism with exponential backoff, the system becomes more resilient to transient network issues and API rate limiting. Furthermore, the adoption of a new batch file upload API and an increased result upload batch size will lead to fewer API calls and faster processing of large data sets, ultimately improving overall performance and reliability. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new withHttpRetry utility function that wraps fetch to provide automatic retries with exponential backoff, jitter, and Retry-After header support for specific HTTP status codes (429, 502, 503). This retry mechanism is then integrated into the main API client. Additionally, the file upload API has been refactored to support batch uploads, allowing multiple files to be sent in a single request to a new /file/batch endpoint. The ResultUploader now groups attachments into batches based on size and count limits before uploading them concurrently. Corresponding test cases for the withHttpRetry utility and updates to existing result upload tests to reflect the new batching behavior were also added. A minor fix was included to round timeTaken to the nearest millisecond in the JUnit XML parser. The reviewer noted that the current batching logic in ResultUploader could still create a batch exceeding MAX_BATCH_SIZE_BYTES if a single attachment is larger than this limit, suggesting an explicit check for individual attachment sizes before batching to prevent API errors and provide clearer feedback.
| for (const item of allAttachments) { | ||
| const size = item.attachment.buffer!.byteLength | ||
| if ( | ||
| currentBatch.length > 0 && | ||
| (currentBatchSize + size > MAX_BATCH_SIZE_BYTES || | ||
| currentBatch.length >= MAX_BATCH_FILE_COUNT) | ||
| ) { | ||
| batches.push(currentBatch) | ||
| currentBatch = [] | ||
| currentBatchSize = 0 | ||
| } | ||
| currentBatch.push(item) | ||
| currentBatchSize += size | ||
| } |
There was a problem hiding this comment.
The current batching logic can create a batch that exceeds MAX_BATCH_SIZE_BYTES if a single attachment is larger than this limit. The check currentBatch.length > 0 allows any file to be added to an empty batch, regardless of its size. This could lead to API errors that are hard to debug.
It would be better to validate the size of each attachment individually before adding it to a batch and provide a clear error message if it's too large.
for (const item of allAttachments) {
const size = item.attachment.buffer!.byteLength
if (size > MAX_BATCH_SIZE_BYTES) {
const errorMsg =
'Attachment "' +
item.attachment.filename +
'" (' +
(size / 1024 / 1024).toFixed(2) +
' MiB) is larger than the maximum batch size of ' +
MAX_BATCH_SIZE_BYTES / 1024 / 1024 +
' MiB.'
if (this.args.force) {
printError(errorMsg + ' Skipping file.')
continue
}
printErrorThenExit(errorMsg)
}
if (
currentBatch.length > 0 &&
(currentBatchSize + size > MAX_BATCH_SIZE_BYTES ||
currentBatch.length >= MAX_BATCH_FILE_COUNT)
) {
batches.push(currentBatch)
currentBatch = []
currentBatchSize = 0
}
currentBatch.push(item)
currentBatchSize += size
}There was a problem hiding this comment.
This size is only for batching opportunity.
If the file is larger than that, we leave it to the server. Currently we don't allow file greater than 50 MB on the server, but if we change that in future, no changes would be required for the CLI tool
1. Implemented retry with exponential backoff in case of 429, 502 and 503 HTTP statuses 2. Increased result upload batch size from 50 to 500 3. Using the newer batch file upload API, which allows max file count of 100 and max cumulative size of 100 MiB. Also reduced the file upload concurrency from 10 to 3
hi-rai
force-pushed
the
dev/himanshu/handle-api-rate-limiting
branch
from
8832450 to
aa969c7
Compare
2 participants
This fixes https://github.com/Hypersequent/tms-issues/issues/2341