-
Notifications
You must be signed in to change notification settings - Fork 933
Add _len_ function to AIOProducer #2140
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
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
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 adds a __len__ method to the AIOProducer class to provide accurate count of all pending messages across both the async batch buffer and librdkafka's output queue. Previously, only messages in librdkafka's queue were counted.
Key changes:
- Implemented
__len__method that sums messages from both queues - Added comprehensive unit tests covering various producer states
- Returns 0 when producer is closed
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/confluent_kafka/experimental/aio/producer/_AIOProducer.py | Added __len__ method implementation with docstring and closed state handling |
| tests/test_AIOProducer.py | Added 4 new test cases covering buffered messages, post-flush state, closed producer, and mixed state scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @pytest.mark.asyncio | ||
| async def test_aio_producer_len_with_buffered_messages(self, mock_producer, mock_common, basic_config): | ||
| """Test that __len__ counts messages in async batch buffer""" | ||
| producer = AIOProducer(basic_config, batch_size=10, buffer_timeout=0) |
Copilot
AI
Nov 27, 2025
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.
The test patches _flush_buffer to prevent auto-flush, but doesn't verify that the patch was actually called or not called. Consider adding an assertion like mock_flush.assert_not_called() after producing messages to ensure the buffer wasn't inadvertently flushed.
| with patch.object(producer, '_flush_buffer') as mock_flush: | ||
| for i in range(7): | ||
| await producer.produce('test-topic', value=f'msg-{i}'.encode()) |
Copilot
AI
Nov 27, 2025
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.
The test patches _flush_buffer but doesn't verify the flushing behavior. With batch_size=5, the buffer should flush after the 5th message. Consider asserting mock_flush.call_count == 1 after the loop to confirm the expected flush occurred.
|
fangnx
left a comment
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.
LGTM




Summary
Added
__len__method toAIOProducerclass to correctly count all pending messages, including those in both the async batch buffer and librdkafka's output queue.Problem
Previously,
len(producer._producer)only counted messages in librdkafka's output queue, missing messages that were still in the async batch buffer waiting to be sent to librdkafka.Solution
Implemented
__len__method that returns the sum of:len(self._producer)self._batch_processor.get_buffer_size()Tests
Unit tests
tests/test_AIOProducer.py- 4 new test cases addedManual test
test_aio_producer_continuous.py