lianetm commented on code in PR #16272:
URL: https://github.com/apache/kafka/pull/16272#discussion_r1638158343
##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumerTest.java:
##########
@@ -158,7 +158,12 @@ public class AsyncKafkaConsumerTest {
public void resetAll() {
backgroundEventQueue.clear();
if (consumer != null) {
- consumer.close(Duration.ZERO);
+ try {
+ consumer.close(Duration.ZERO);
+ } catch (Exception e) {
+ // best effort to clean up after each test, but may throw (ex.
if callbacks where
+ // throwing errors)
Review Comment:
Oh I see, good point. Agree with going with a consistent approach here, but
could I bring it in a follow-up PR right after this (it wouldn't need to get
into 3.8). The trick is that now the close may throw for any of the tests that
throw on the callback, so I would prefer to play safe, make sure the close does
not throw here, and I'll review all the tests that we expect could rightfully
throw on close now (Also the other clean-up logic has some Timeout related
checks so better not removing blindly, could have some value it seems)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
