appchemist commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2159551660
Thanks for review @lucasbru
Thank you again for taking the time to review. @philipnee @lianetm @kirktrue
--
This is an automated message from the Apache Git Service.
To respond
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1634006443
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -199,4 +234,4 @@ public void
lucasbru commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1633531270
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -199,4 +234,4 @@ public void
lucasbru merged PR #16043:
URL: https://github.com/apache/kafka/pull/16043
--
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:
kirktrue commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1633528798
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -199,4 +234,4 @@ public void
appchemist commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2155732666
@philipnee & @lianetm Thanks for review!
--
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
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631473205
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +180,38 @@ public void testHasAnyPendingRequests()
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631469076
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +180,38 @@ public void
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631466015
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +180,38 @@ public void testHasAnyPendingRequests()
lianetm commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2155187907
Thanks for the changes @appchemist ! Could we update the PR description to:
- remove the "... a new PR has been created" line. (the other PR is linked
in many places throughout the
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631458459
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -83,12 +84,14 @@
import static
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631458459
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -83,12 +84,14 @@
import static
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631455569
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -83,12 +84,14 @@
import static org.mockito.Mockito.verify;
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631404977
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerSubscriptionTest.scala:
##
@@ -223,6 +225,27 @@ class PlaintextConsumerSubscriptionTest extends
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631404977
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerSubscriptionTest.scala:
##
@@ -223,6 +225,27 @@ class PlaintextConsumerSubscriptionTest extends
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631410054
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +184,63 @@ public void testHasAnyPendingRequests()
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631404977
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerSubscriptionTest.scala:
##
@@ -223,6 +225,27 @@ class PlaintextConsumerSubscriptionTest extends
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631404294
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +184,63 @@ public void
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631397697
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerSubscriptionTest.scala:
##
@@ -223,6 +225,27 @@ class PlaintextConsumerSubscriptionTest extends
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631396237
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +184,63 @@ public void testHasAnyPendingRequests()
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631395106
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +184,63 @@ public void testHasAnyPendingRequests()
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631395106
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +184,63 @@ public void testHasAnyPendingRequests()
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631394278
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -360,6 +367,18 @@ void testSendUnsentRequest() {
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631389671
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +184,63 @@ public void testHasAnyPendingRequests()
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631386757
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +184,63 @@ public void testHasAnyPendingRequests()
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631385696
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -360,6 +367,18 @@ void testSendUnsentRequest() {
appchemist commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2155022679
@philipnee & @lianetm Thanks for review!
--
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
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631337110
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -189,6 +189,27 @@ class PlaintextConsumerTest extends BaseConsumerTest {
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631337110
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -189,6 +189,27 @@ class PlaintextConsumerTest extends BaseConsumerTest {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631330338
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -192,15 +192,22 @@ class PlaintextConsumerTest extends BaseConsumerTest {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631328022
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -192,15 +192,22 @@ class PlaintextConsumerTest extends BaseConsumerTest {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631326145
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -192,15 +192,22 @@ class PlaintextConsumerTest extends BaseConsumerTest {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631322984
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -192,15 +192,22 @@ class PlaintextConsumerTest extends BaseConsumerTest {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631262656
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -189,6 +189,20 @@ class PlaintextConsumerTest extends BaseConsumerTest {
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631268525
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -189,6 +189,20 @@ class PlaintextConsumerTest extends BaseConsumerTest {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631262656
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -189,6 +189,20 @@ class PlaintextConsumerTest extends BaseConsumerTest {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631236747
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -189,6 +189,20 @@ class PlaintextConsumerTest extends BaseConsumerTest {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631236747
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -189,6 +189,20 @@ class PlaintextConsumerTest extends BaseConsumerTest {
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631102468
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +183,80 @@ public void
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1631101468
##
core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala:
##
@@ -189,6 +189,20 @@ class PlaintextConsumerTest extends BaseConsumerTest {
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630919078
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThread.java:
##
@@ -145,6 +154,7 @@ void runOnce() {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630912266
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -360,6 +370,52 @@ void testSendUnsentRequest() {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630911599
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -360,6 +370,52 @@ void testSendUnsentRequest() {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630908222
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +183,80 @@ public void testHasAnyPendingRequests()
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630904040
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +183,80 @@ public void testHasAnyPendingRequests()
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630902382
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/NetworkClientDelegateTest.java:
##
@@ -169,14 +183,80 @@ public void testHasAnyPendingRequests()
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630893130
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -360,6 +370,52 @@ void testSendUnsentRequest() {
appchemist closed pull request #15961: KAFKA-16764: New consumer should throw
InvalidTopicException on poll when invalid topic in metadata.
URL: https://github.com/apache/kafka/pull/15961
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to
appchemist commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2153682806
@philipnee right, I got it
--
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
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630477551
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThread.java:
##
@@ -145,6 +154,7 @@ void runOnce() {
philipnee commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2153668330
hey @appchemist - thanks for putting time into the pr. i assume
[#16043](https://github.com/apache/kafka/pull/16043) is the pr you want us to
review, no? it would be easier to just
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630477551
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThread.java:
##
@@ -145,6 +154,7 @@ void runOnce() {
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630472373
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThread.java:
##
@@ -145,6 +154,7 @@ void runOnce() {
appchemist commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1630461949
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MetadataErrorManager.java:
##
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software
lianetm commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2153385925
Hey @appchemist, thanks for the changes! Seems like a simpler approach. Left
one question about the specific place to have the logic. Thanks!
--
This is an automated message from the
lianetm commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1630221371
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThread.java:
##
@@ -145,6 +154,7 @@ void runOnce() {
philipnee commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1629846782
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MetadataErrorManager.java:
##
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software
appchemist commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1629752083
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MetadataErrorManager.java:
##
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software
appchemist opened a new pull request, #15961:
URL: https://github.com/apache/kafka/pull/15961
- Add ErrorPropagateMetadataUpdater
- Just proxy but propagates error though BackgroundEventHandler
### Committer Checklist (excluded from commit message)
- [ ] Verify design and
philipnee commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1629622755
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MetadataErrorManager.java:
##
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software
appchemist commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2152367583
@philipnee Thanks for respond
1. No problem, I misunderstood.
2. Implemented in
`ConsumerNetworkThreadTest`(https://github.com/apache/kafka/pull/16043)
In my thought,
appchemist commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2151916716
If you have a moment, Please take a look. @lianetm , @philipnee , @kirktrue
I've applied a simple approach to the ConsumerNetworkThread level.
I pushed it a few minutes ago,
appchemist commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2151383230
Reopen this pr for KAFKA-16764
--
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
appchemist closed pull request #15961: KAFKA-16764: New consumer should throw
InvalidTopicException on poll when invalid topic in metadata.
URL: https://github.com/apache/kafka/pull/15961
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to
appchemist commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1628553681
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MetadataErrorManager.java:
##
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software
appchemist commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1628553681
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MetadataErrorManager.java:
##
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software
kirktrue commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2151071098
I think that the approach from #16043 is more in line with the approach I
imagined.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log
appchemist commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1628496087
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MetadataErrorManager.java:
##
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software
philipnee commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2151036734
@appchemist - was away for a few days so sorry for not responding. there
are two points to address before moving the pr forward
1. we definitely don't need to implement the request
appchemist commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2151024735
@lianetm No problem, I enjoyed the discussion.
@kirktrue Thanks for review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on
philipnee commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1628483904
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MetadataErrorManager.java:
##
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software
lianetm commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2150713653
Hey @kirktrue , before continuing reviewing this PR, we had some concerns
about the approach, and were considering a simpler alternative, see [comment
kirktrue commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1628244915
##
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MetadataErrorManager.java:
##
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation
lianetm commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2150487736
Oh I totally understand you now with that other PR and the feedback there. I
see @philipnee initially suggested the managers approach but I believe we've
come to the point after the
appchemist commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2150328159
Hey @lianetm
Sorry about that. I've created a Simple version of the PR separately, and I
think that's why it didn't share.
Firstly, I understand the differences you
lianetm commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2149990136
Hey @appchemist, agree that as long as we continue polling the
ConsumerNetworkThread we should get the error thrown (on the next poll after it
was detected), but the inverted order we
appchemist commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2149560613
@lianetm Thanks for review!
I agree that the Manager approach is a bit like an overkill.
For issues that have different timing to legacy consumers, I think about it
like
lianetm commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2148155631
Hey all, I still have concerns with this approach of a manager just to
propagate the metadata errors. Not only that it still feels a bit like an
overkill, but I'm worried it brings a
appchemist commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2147617237
@philipnee if you have a moment, please take a look
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL
appchemist commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1620884853
##
clients/src/main/java/org/apache/kafka/clients/ClientUtils.java:
##
@@ -172,7 +173,37 @@ public static NetworkClient
createNetworkClient(AbstractConfig config,
appchemist commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1618318070
##
clients/src/main/java/org/apache/kafka/clients/ClientUtils.java:
##
@@ -172,7 +173,37 @@ public static NetworkClient
createNetworkClient(AbstractConfig config,
appchemist commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2136860702
To summarize, the locations where Metadata Errors can be handled, it seems
like three main options exist:
- `ConsumerNetworkThread`
- `NetworkClientDelegate`
-
appchemist commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1618318070
##
clients/src/main/java/org/apache/kafka/clients/ClientUtils.java:
##
@@ -172,7 +173,37 @@ public static NetworkClient
createNetworkClient(AbstractConfig config,
philipnee commented on code in PR #15961:
URL: https://github.com/apache/kafka/pull/15961#discussion_r1618032800
##
clients/src/main/java/org/apache/kafka/clients/ClientUtils.java:
##
@@ -172,7 +173,37 @@ public static NetworkClient
createNetworkClient(AbstractConfig config,
lianetm commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2135478481
Hey @appchemist , thanks for the updates! High level comment with the goal
of trying to simplify if possible. I don't quite get the need for a new way of
updating metadata
appchemist commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2134918869
kindly ping @philipnee
--
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
appchemist closed pull request #16043: KAFKA-16764: New consumer should throw
InvalidTopicException on poll when invalid topic in metadata
URL: https://github.com/apache/kafka/pull/16043
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to
appchemist commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2130614939
Hey @philipnee
I got it!
I think you're confused because I created two PRs.
In my think, this PR is far from our priorities, so I'll close it.
I'm sorry, but could you
philipnee commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2130547360
Hey @appchemist -- I mistakenly thought this PR was #15961, sorry. if you
read ConsumerNetwokrThread#runOnce() you can see the original intention was to
explicitly define and execute
appchemist commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2128669455
> i really want to keep the network client delegate to just interfacing with
the network client, so i wonder if it would be a better design to handle the
metadata error in a separated
appchemist commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2128663846
Hi @philipnee, Thank you for review!
> do you think we can put the metadata error in the ConsumerNetworkThread
and have an exception handler to relay the error back to the user
philipnee commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2127634542
hey @appchemist - thanks for the patch. do you think we can put the
metadata error in the ConsumerNetworkThread and have an exception handler to
relay the error back to the user via
philipnee commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1612009583
##
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##
@@ -2952,8 +2952,8 @@ private static class FetchInfo {
// TODO: this test
appchemist commented on PR #16043:
URL: https://github.com/apache/kafka/pull/16043#issuecomment-2127053686
@philipnee & @kirktrue If you have a moment, Please take a look
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and
appchemist commented on code in PR #16043:
URL: https://github.com/apache/kafka/pull/16043#discussion_r1611653408
##
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##
@@ -2974,6 +2974,7 @@ public void testSubscriptionOnInvalidTopic(GroupProtocol
appchemist opened a new pull request, #16043:
URL: https://github.com/apache/kafka/pull/16043
Propagate metadata error from background thread to foreground thread through
backgroundEventQueue.
Due to a different approach from https://github.com/apache/kafka/pull/15961,
a new PR has
appchemist commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2125950049
@lianetm I got it, Thank you!
@kirktrue Right! I think what you said is simple and more intuitive.
--
This is an automated message from the Apache Git Service.
To respond to the
lianetm commented on PR #15961:
URL: https://github.com/apache/kafka/pull/15961#issuecomment-2125262436
Hey @appchemist, thanks for the updates! sorry I'm caught up in other tasks
but we discussed offline and @philipnee will follow-up here to help with
reviews on this one. Thanks!
--
98 matches
Mail list logo
