Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-11-02 Thread via GitHub
lucasbru merged PR #14639: URL: https://github.com/apache/kafka/pull/14639 -- 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

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-11-02 Thread via GitHub
lucasbru commented on PR #14639: URL: https://github.com/apache/kafka/pull/14639#issuecomment-1790316683 Test failures unrelated -- 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 comme

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-11-01 Thread via GitHub
philipnee commented on PR #14639: URL: https://github.com/apache/kafka/pull/14639#issuecomment-1789989071 Test failures seem irrelevant, but retriggering the test as there's a failed build -- This is an automated message from the Apache Git Service. To respond to the message, please log o

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-30 Thread via GitHub
philipnee commented on PR #14639: URL: https://github.com/apache/kafka/pull/14639#issuecomment-1785408525 @lucasbru - Thanks for taking time reviewing this PR. I addressed your previous comment. Let me know if there's anything unclear on this PR. -- This is an automated message from the A

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-30 Thread via GitHub
philipnee commented on code in PR #14639: URL: https://github.com/apache/kafka/pull/14639#discussion_r1376339956 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java: ## @@ -215,16 +296,117 @@ public NetworkClientDelegate.UnsentRequest

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-30 Thread via GitHub
lucasbru commented on code in PR #14639: URL: https://github.com/apache/kafka/pull/14639#discussion_r1375757133 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java: ## @@ -215,16 +296,117 @@ public NetworkClientDelegate.UnsentRequest t

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-29 Thread via GitHub
philipnee commented on PR #14639: URL: https://github.com/apache/kafka/pull/14639#issuecomment-1784489022 Hi @lucasbru - Thanks for the response. To your first question: Yes, what is left is correcting the error handling at the user API level. At the time I implemented this, I wrapped all

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-27 Thread via GitHub
lucasbru commented on PR #14639: URL: https://github.com/apache/kafka/pull/14639#issuecomment-1782544764 Changes look good to me. Did I understand david correctly that we want to add more changes to this PR? > 1. Mockito: Could you be more specific on how you expect to mock the respo

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-27 Thread via GitHub
lucasbru commented on code in PR #14639: URL: https://github.com/apache/kafka/pull/14639#discussion_r1374244715 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java: ## @@ -271,14 +445,13 @@ public void onResponse( private voi

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-26 Thread via GitHub
philipnee commented on PR #14639: URL: https://github.com/apache/kafka/pull/14639#issuecomment-1782275615 Hi @lucasbru - Thanks for taking the time to review my PR. I addressed all but 2 comments: 1. Mockito: Could you be more specific on how you expect to mock the response object? 2

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-26 Thread via GitHub
philipnee commented on code in PR #14639: URL: https://github.com/apache/kafka/pull/14639#discussion_r1374057359 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java: ## @@ -127,7 +146,7 @@ public void maybeAutoCommit(final Map offsets

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-26 Thread via GitHub
philipnee commented on code in PR #14639: URL: https://github.com/apache/kafka/pull/14639#discussion_r1374040965 ## clients/src/test/java/org/apache/kafka/clients/consumer/internals/CommitRequestManagerTest.java: ## @@ -382,9 +522,49 @@ private ClientResponse buildOffsetFetchCli

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-26 Thread via GitHub
philipnee commented on code in PR #14639: URL: https://github.com/apache/kafka/pull/14639#discussion_r1373587460 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java: ## @@ -215,16 +244,115 @@ public NetworkClientDelegate.UnsentRequest

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-26 Thread via GitHub
philipnee commented on code in PR #14639: URL: https://github.com/apache/kafka/pull/14639#discussion_r1373585188 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java: ## @@ -215,16 +244,115 @@ public NetworkClientDelegate.UnsentRequest

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-26 Thread via GitHub
philipnee commented on code in PR #14639: URL: https://github.com/apache/kafka/pull/14639#discussion_r1373567351 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java: ## @@ -215,16 +244,115 @@ public NetworkClientDelegate.UnsentRequest

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-26 Thread via GitHub
lucasbru commented on PR #14639: URL: https://github.com/apache/kafka/pull/14639#issuecomment-1781118953 Thanks @philipnee, I left some comments -- 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 th

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-26 Thread via GitHub
lucasbru commented on code in PR #14639: URL: https://github.com/apache/kafka/pull/14639#discussion_r1373051849 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java: ## @@ -148,28 +167,35 @@ Queue unsentOffsetCommitRequests() {

Re: [PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-26 Thread via GitHub
scyber commented on code in PR #14639: URL: https://github.com/apache/kafka/pull/14639#discussion_r1372704905 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/CommitRequestManager.java: ## @@ -215,16 +227,113 @@ public NetworkClientDelegate.UnsentRequest toU

[PR] KAFKA-15562: ensure commit request manager handles errors correctly [kafka]

2023-10-25 Thread via GitHub
philipnee opened a new pull request, #14639: URL: https://github.com/apache/kafka/pull/14639 *More detailed description of your change, if necessary. The PR title and PR message become the squashed commit message, so use a separate comment to ping reviewers.* *Summary of testi