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 t
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 prepareFindCoordinatorRespons
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 prepareFindCoordinatorResponse(
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: jira-unsubscr...@kafka.apache
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 prepareFindCoordinatorResponse(
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 to
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() th
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 testHasAnyPendingRequests()
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() th
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 disc
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 org.mockito.Mockito.verify
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 org.mockito.Mockito.verify
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
Abstra
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
Abstra
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
Abstra
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 testHasAnyPendingRequests()
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
AbstractC
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() th
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() th
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() {
a
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() th
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() th
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() {
ass
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 to
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 {
assertThro
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 {
assertThro
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 {
@Paramete
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 {
@Paramete
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 {
@Paramete
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 {
@Paramete
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 {
assertTh
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 {
assertT
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 {
assertTh
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 {
assertTh
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 {
assertTh
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 testHasAnyPendingRequests()
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 {
assertT
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() {
.map(networ
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() {
a
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() {
a
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() {
a
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() {
.map(networ
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 hav
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() {
.map(networ
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() {
.map(network
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 Foundatio
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 Ap
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() {
.map(networkCl
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 Foundation
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 Foundatio
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 impl
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 Foundation
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, p
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, s
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 speci
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 Foundatio
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 Foundatio
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 o
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 Foundatio
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 m
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 to
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 Foundation
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
above](https://github.com/apache/kaf
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 discu
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 mentio
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 get
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 t
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 fund
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 abo
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`
- `KafkaClient
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 (`ErrorPropagat
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 com
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 G
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 pl
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 t
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 v
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 th
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 r
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 use
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 b
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 me
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!
--
Thi
98 matches
Mail list logo
