Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
lucasbru merged PR #15576: URL: https://github.com/apache/kafka/pull/15576 -- 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
Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
philipnee commented on PR #15576: URL: https://github.com/apache/kafka/pull/15576#issuecomment-2021140028 @lucasbru @kirktrue - Sorry for the oversight. I pushed out the changes to address Kirk's comment. -- 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
Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
lucasbru commented on PR #15576: URL: https://github.com/apache/kafka/pull/15576#issuecomment-2019978674 @philipnee Could you please address/respond to @kirktrue's comments. We seem to be dropping parameters here, and some test cases with the new coordinator seem to be duplicated after the change. -- 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
Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
philipnee commented on PR #15576: URL: https://github.com/apache/kafka/pull/15576#issuecomment-2019090272 @kirktrue @lucasbru - mind taking a second look? For this specific PR I only modified the tests using console consumer that's why several tests cases were omitted. -- 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
Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
vamossagar12 commented on PR #15576: URL: https://github.com/apache/kafka/pull/15576#issuecomment-2017572320 @philipnee , here's the PR that I created: https://github.com/apache/kafka/pull/15594. Actually I looked at your changes now and it appears that the changes in the new PR consists of all the changes in this PR plus additional connect changes. Let me know if we can continue working on the new one? I will loop in connect experts as well once the basic structure is reviewed. -- 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
Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
philipnee commented on PR #15576: URL: https://github.com/apache/kafka/pull/15576#issuecomment-2015459852 Thanks @vamossagar12 - You can either create a new jira ticket for fixing these connectors or using the existing tickets. Maybe we should create a new ticket for new connector interface in the system tests as it is a bit more involving than just parametrizing the tests. -- 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
Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
vamossagar12 commented on PR #15576: URL: https://github.com/apache/kafka/pull/15576#issuecomment-2015385828 @philipnee , thanks .ok will continue. Yeah I can send out a pr next week for this. -- 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
Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
philipnee commented on PR #15576: URL: https://github.com/apache/kafka/pull/15576#issuecomment-2015334416 @vamossagar12 - Hey - The connector test still requires a bit of parametrization on the connector side. If you feel like you have the bandwidth to handle this feel free to continue with the work. The current state of PR only parameterizes the test that are using the consumer directly. -- 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
Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
philipnee commented on PR #15576: URL: https://github.com/apache/kafka/pull/15576#issuecomment-2015330638 Hey @kirktrue - Thanks for the review. I didn't put in the parameters because these tests don't configure the consumer directly. I think if can modify the connector to use the latest consumer, then it would make sense to put in these params. -- 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
Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
vamossagar12 commented on PR #15576: URL: https://github.com/apache/kafka/pull/15576#issuecomment-2014406407 @philipnee , I had assigned the ticket to myself and had started working on it (was in the process of testing it actually). Anyways since you have the PR, so you would want to continue on this then? -- 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
Re: [PR] KAFKA-16272: Update connect_distributed_test.py to support KIP-848’s group protocol config [kafka]
kirktrue commented on code in PR #15576: URL: https://github.com/apache/kafka/pull/15576#discussion_r1535030295 ## tests/kafkatest/tests/connect/connect_distributed_test.py: ## @@ -692,7 +692,12 @@ def test_file_source_and_sink(self, security_protocol, exactly_once_source, conn metadata_quorum=[quorum.isolated_kraft], use_new_coordinator=[True, False] ) -def test_bounce(self, clean, connect_protocol, metadata_quorum, use_new_coordinator=False): +@matrix( +metadata_quorum=[quorum.isolated_kraft], +use_new_coordinator=[True], +group_protocol=consumer_group.all_group_protocols +) Review Comment: We need the `clean` and `connect_protocol` values to be included. ```suggestion @matrix( clean=[True, False], connect_protocol=['sessioned', 'compatible', 'eager'], metadata_quorum=[quorum.isolated_kraft], use_new_coordinator=[True], group_protocol=consumer_group.all_group_protocols ) ``` ## tests/kafkatest/tests/connect/connect_distributed_test.py: ## @@ -929,7 +940,12 @@ def test_exactly_once_source(self, clean, connect_protocol, metadata_quorum, use metadata_quorum=[quorum.isolated_kraft], use_new_coordinator=[True, False] Review Comment: ```suggestion use_new_coordinator=[False] ``` ## tests/kafkatest/tests/connect/connect_distributed_test.py: ## @@ -929,7 +940,12 @@ def test_exactly_once_source(self, clean, connect_protocol, metadata_quorum, use metadata_quorum=[quorum.isolated_kraft], use_new_coordinator=[True, False] ) -def test_transformations(self, connect_protocol, metadata_quorum, use_new_coordinator=False): +@matrix( +metadata_quorum=[quorum.isolated_kraft], +use_new_coordinator=[True], +group_protocol=consumer_group.all_group_protocols +) Review Comment: We still need the `connect_protocol` here: ```suggestion @matrix( connect_protocol=['sessioned', 'compatible', 'eager'], metadata_quorum=[quorum.isolated_kraft], use_new_coordinator=[True], group_protocol=consumer_group.all_group_protocols ) ``` ## tests/kafkatest/tests/connect/connect_distributed_test.py: ## @@ -823,6 +829,11 @@ def test_bounce(self, clean, connect_protocol, metadata_quorum, use_new_coordina metadata_quorum=[quorum.isolated_kraft], use_new_coordinator=[True, False] ) +@matrix( +metadata_quorum=[quorum.isolated_kraft], +use_new_coordinator=[True], +group_protocol=consumer_group.all_group_protocols +) Review Comment: Either we need to add the `group_protocol` parameter, or else just remove this new `@matrix`. ```suggestion ``` -- 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
