-
Notifications
You must be signed in to change notification settings - Fork 23
feat: Add configurable maxBatchSize
to consumer.run()
#388
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: master
Are you sure you want to change the base?
Conversation
Allow users to configure the maximum batch size when using eachBatch handler instead of the hardcoded value of 32. Changes: - Add maxBatchSize parameter to ConsumerRunConfig type - Update run() method to accept and validate maxBatchSize - Default to 32 to maintain backwards compatibility - Validate that maxBatchSize is a positive integer - Update #runInternal to use configurable value - Add JSDoc documentation for the new parameter - Add comprehensive test coverage for the feature The maxBatchSize parameter controls how many messages are included in a single batch when using the eachBatch handler. Users can now tune this based on their message size and processing requirements. Tests verify default behavior, custom values, validation, large batch sizes, and that the parameter only affects eachBatch (not eachMessage).
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
Adds a configurable maxBatchSize option to consumer.run(), allowing users of eachBatch to control maximum batch size instead of the previously hardcoded 32.
- Introduces maxBatchSize validation and defaulting logic (intended default 32)
- Propagates maxBatchSize into #runInternal and updates tests to cover default, custom, large values, validation errors, and isolation from eachMessage
- Adds new tests validating behavior and error scenarios
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
lib/kafkajs/_consumer.js | Adds JSDoc param, validation, and replaces hardcoded batch size with configurable value |
test/promisified/consumer/consumeMessages.spec.js | Adds test cases for default/custom/large maxBatchSize, validation failures, and ensuring eachMessage unaffected |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Allow users to configure the maximum batch size when using eachBatch handler instead of the hardcoded value of 32. Changes: - Add maxBatchSize parameter to ConsumerRunConfig type - Update run() method to accept and validate maxBatchSize - Default to 32 to maintain backwards compatibility - Validate that maxBatchSize is a positive integer - Update #runInternal to use configurable value - Add JSDoc documentation for the new parameter - Add comprehensive test coverage for the feature The maxBatchSize parameter controls how many messages are included in a single batch when using the eachBatch handler. Users can now tune this based on their message size and processing requirements. Tests verify default behavior, custom values, validation, large batch sizes, and that the parameter only affects eachBatch (not eachMessage).
Allow users to configure the maximum batch size when using eachBatch handler instead of the hardcoded value of 32.
Changes:
The maxBatchSize parameter controls how many messages are included in a single batch when using the eachBatch handler. Users can now tune this based on their message size and processing requirements.
Tests verify default behavior, custom values, validation, large batch sizes, and that the parameter only affects eachBatch (not eachMessage).
Checklist
References
Addresses #286
Test & Review
Unit tested
Open questions / Follow-ups
This is my first PR on this repo, so any advice is welcome.
Review stakeholders
Probably @milindl since git blame points to him on the TODO.