vvcephei commented on a change in pull request #10022:
URL: https://github.com/apache/kafka/pull/10022#discussion_r569635435
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
##########
@@ -1236,7 +1236,7 @@ public void assign(Collection<TopicPartition> partitions)
{
}
final FetchedRecords<K, V> records = pollForFetches(timer);
- if (!records.isEmpty()) {
+ if (!records.records().isEmpty()) {
Review comment:
Ok, @chia7712 and @rajinisivaram , I've restarted the VOTE thread with a
new message.
Hopefully, we can wrap up that discussion quickly, so I can circle back to
either change the feature or the tests.
Thanks for that counter-example, @chia7712 ! Actually, we were aware of that
kind of case, and your proposed workaround is exactly what we had to do in the
integration tests:
https://github.com/apache/kafka/pull/9836/files#diff-735dcc2179315ebd78a7c75fd21b70b0ae81b90f3d5ec761740bc80abeae891fR1875-R1888
:)
The key question, which I tried to pose in the mailing list, is whether this
is really a "real" use case we have to support, or whether it's just something
we happen to do in some tests or are able to imagine. We can certainly add a
new method to the interface, but that also has nontrivial usability costs, as
users need to understand the differences of those two methods and we also have
to maintain and test both code paths. If it's not that likely that someone
outside of our own project will be harmed, it seems better to just make the
change in place.
Anyway, we should discuss on the mailing list; I just wanted to acknowledge
your response.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
