You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/07/06 18:39:41 UTC
[GitHub] [kafka] kirktrue opened a new pull request #10980: KAFKA-12989: MockClient should respect the request matcher passed to prepareUnsupportedVersionResponse
kirktrue opened a new pull request #10980:
URL: https://github.com/apache/kafka/pull/10980
Handle the case where `matches` returns `false` and throw the `InvalidStateException` as stated by the JavaDoc.
We need to guard against this unexpected runtime error in the `KafkaAdminClient`'s `sendEligibleCalls` method with a try/catch. Not 100% sure if that's kosher or not.
Included a targeted unit test for this case. The remaining tests in `KafkaAdminTestClient` continue to pass.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue commented on pull request #10980: KAFKA-12989: MockClient should respect the request matcher passed to prepareUnsupportedVersionResponse
Posted by GitBox <gi...@apache.org>.
kirktrue commented on pull request #10980:
URL: https://github.com/apache/kafka/pull/10980#issuecomment-875058699
@hachikuji - would you be willing to assign a reviewer for this PR?
The failing tests look like they're related to KRaft tests, so I don't _think_ they're related.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #10980: KAFKA-12989: MockClient should respect the request matcher passed to prepareUnsupportedVersionResponse
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #10980:
URL: https://github.com/apache/kafka/pull/10980#issuecomment-887020995
Why is `NetworkClient#send` throwing an exception? It shouldn't be doing that, right? Can you explain more about the problem that this PR fixes?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue removed a comment on pull request #10980: KAFKA-12989: MockClient should respect the request matcher passed to prepareUnsupportedVersionResponse
Posted by GitBox <gi...@apache.org>.
kirktrue removed a comment on pull request #10980:
URL: https://github.com/apache/kafka/pull/10980#issuecomment-874998780
Can someone from @confluentinc/clients review, please?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue edited a comment on pull request #10980: KAFKA-12989: MockClient should respect the request matcher passed to prepareUnsupportedVersionResponse
Posted by GitBox <gi...@apache.org>.
kirktrue edited a comment on pull request #10980:
URL: https://github.com/apache/kafka/pull/10980#issuecomment-887635572
> Why is `NetworkClient#send` throwing an exception? It shouldn't be doing that, right? Can you explain more about the problem that this PR fixes?
Per [the original issue](https://issues.apache.org/jira/browse/KAFKA-12989) the `MockClient` used for testing allows for fault injection via the `RequestMatcher`. If the test sets up the condition where the request _doesn't_ match some condition, the `MockClient.send` method is supposed to throw an `IllegalStateException`.
That change seemed straightforward except that this now caused problems in `KafkaAdminClient`. Because it's not expecting any errors, this exception causes the thread in `KafkaAdminClient.sendEligibleCalls` that is servicing requests to die, hence my addition of the `try`/`catch` wrapper.
That said, I'm not 100% confident that this change is the right way to handle things. Please advise.
Thanks!
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue commented on pull request #10980: KAFKA-12989: MockClient should respect the request matcher passed to prepareUnsupportedVersionResponse
Posted by GitBox <gi...@apache.org>.
kirktrue commented on pull request #10980:
URL: https://github.com/apache/kafka/pull/10980#issuecomment-876583510
@cmccabe - could you take a look at this and/or assign a reviewer for this? Thanks!
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue commented on pull request #10980: KAFKA-12989: MockClient should respect the request matcher passed to prepareUnsupportedVersionResponse
Posted by GitBox <gi...@apache.org>.
kirktrue commented on pull request #10980:
URL: https://github.com/apache/kafka/pull/10980#issuecomment-887635572
> Why is `NetworkClient#send` throwing an exception? It shouldn't be doing that, right? Can you explain more about the problem that this PR fixes?
Per [the original issue](https://issues.apache.org/jira/browse/KAFKA-12989) the `MockClient` used for testing allows for fault injection via the `RequestMatcher`. If the test sets up the condition where the request _doesn't_ match some condition, the `MockClient.send` method throws an `IllegalStateException`. Because it's not expecting any errors, this exception causes the thread in `KafkaAdminClient` that is servicing requests to die, hence the `try`/`catch` wrapper.
That said, I'm not 100% confident that this change is the right way to handle things.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue commented on pull request #10980: KAFKA-12989: MockClient should respect the request matcher passed to prepareUnsupportedVersionResponse
Posted by GitBox <gi...@apache.org>.
kirktrue commented on pull request #10980:
URL: https://github.com/apache/kafka/pull/10980#issuecomment-874998780
Can someone from @confluentinc/clients review, please?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org