feat(services): upload strategy internal internal handler#190
feat(services): upload strategy internal internal handler#190
Conversation
… flushAndPush - Add handler-based constructor to StreamDocumentUploadQueue - Add no-arg constructor for test compatibility - Extract clearQueue() private method - Add flushAndPush() method with handler delegation - Maintain backward compatibility with uploader-based path
- Remove fileContainer management from UpdateStreamService - Remove fileContainer management from UpdateStreamServiceInternal - Create CatalogStreamUploadHandler in UpdateStreamService constructor - Use handler-based queue factory method forStreamSource() - Delegate document operations to queue instead of managing containers - Simplify close() to call queue.flushAndPush() directly - Handler now owns file container lifecycle (separation of concerns)
…ecture - Update StreamDocumentUploadQueueTest: mock UploadStrategy<StreamUpdate> - Add test for handler-based constructor path - Update UpdateStreamServiceInternalTest: remove file container tests - Remove obsolete tests for createUploadAndPush() method - Update tests to validate queue delegation pattern - All 154 tests pass with new handler-based queue architecture
y-lakhdar
changed the title
y-lakhdar
changed the base branch from
main
to
feat/configurable-batch-size-services
y-lakhdar
changed the base branch from
feat/configurable-batch-size-services
to
main
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Clear lastResponse when flush skips an empty batch
When flush() exits early on an empty queue, it leaves lastResponse unchanged. Because UpdateStreamServiceInternal.close() returns queue.getLastResponse(), a close() called after an auto-flush (where the queue is empty) will return the previous batch’s response even though no push occurred during the close. This makes it impossible for callers to distinguish a no-op close from a successful final push and can mask missing final uploads. Consider resetting lastResponse to null when flush() returns early (or when clearing the queue) so close() accurately reflects whether a push happened.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4 participants
Summary
Adds configurable batch size support to all push services and implements per-batch file container rotation for UpdateStreamService following Coveo's catalog stream API best practices.
Key Changes
Configurable Batch Size
All services now support custom batch sizes (max: 256MB):
PushService(source, options, maxQueueSize)StreamService(source, options, userAgents, maxQueueSize)UpdateStreamService(source, options, userAgents, maxQueueSize)File Container Rotation (UpdateStreamService)
Each batch now gets its own file container:
This follows the catalog stream API best practices.
Testing
Part 3 of 3 - Split from PR #190