[GitHub] [kafka] mimaison commented on pull request #11288: MINOR: Fix error response generation
mimaison commented on pull request #11288: URL: https://github.com/apache/kafka/pull/11288#issuecomment-913647816 Backports: - 2.8: https://github.com/apache/kafka/pull/11299 - 3.0: https://github.com/apache/kafka/pull/11300 -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] mimaison commented on pull request #11288: MINOR: Fix error response generation
mimaison commented on pull request #11288: URL: https://github.com/apache/kafka/pull/11288#issuecomment-912527113 Thanks @dajac for the feedback. I've addressed your comments. I agree with your comment about testing of these classes. `RequestResponseTest` is messy and it's hard to tell what's actually tested. I wouldn't be surprized if some requests or responses are completely missed! -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] mimaison commented on pull request #11288: MINOR: Fix error response generation
mimaison commented on pull request #11288: URL: https://github.com/apache/kafka/pull/11288#issuecomment-910126913 KAFKA-13260 is a regression in 3.0. However, I'm not sure if it actually has any impact. Looking at all callers of `errorCounts()`, it looks like it's mostly used to check for `NOT_CONTROLLER` error which can't happen with `FindCoordinator`. KAFKA-13258 is a regression in 2.8. This is directly visible by users when calling `Admin.alterClientQuotas()`, so I consider this high impact. KAFKA-13259 is here since the `DescribeProducers` API was introduced in 2.8. But `Admin.describeProducers()` is only being added in 3.0. I've not tried explicitly with a 3.0 client but I expect it behaves like KAFKA-13258, ie you don't get an Exception even when the call fails, so it's also high impact. @kkonstantine can you take a look an decide whether we want this PR in 3.0? KAFKA-13258 and KAFKA-13259 are not technically regressions in 3.0 but are pretty annoying for users. -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] mimaison commented on pull request #11288: MINOR: Fix error response generation
mimaison commented on pull request #11288: URL: https://github.com/apache/kafka/pull/11288#issuecomment-910053809 -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] mimaison commented on pull request #11288: MINOR: Fix error response generation
mimaison commented on pull request #11288: URL: https://github.com/apache/kafka/pull/11288#issuecomment-910086728 I opened: - https://issues.apache.org/jira/browse/KAFKA-13258 - https://issues.apache.org/jira/browse/KAFKA-13259 - https://issues.apache.org/jira/browse/KAFKA-13260 We can't easily split the PR as the test makes them all fail -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] mimaison commented on pull request #11288: MINOR: Fix error response generation
mimaison commented on pull request #11288: URL: https://github.com/apache/kafka/pull/11288#issuecomment-910060115 Yes it's partly a regression: - AlterClientQuotas worked correctlyup to 2.7.0 and is broken since 2.8.0. - FindCoordinator worked correctly up to 2.8.0 and is broken since 3.0.0 DescribeProducers is broken since releasing in 2.8.0. It almost feels like we should create a ticket (and PR) for each of them -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] mimaison commented on pull request #11288: MINOR: Fix error response generation
mimaison commented on pull request #11288: URL: https://github.com/apache/kafka/pull/11288#issuecomment-910053809 This affects 2.8.0 too so part of this PR will need to be backported. -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
