nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2084760582
@chia7712 Thank you for the review and merge!
Will prepare next test of ConfigCommand for review shortly.
--
This is an automated message from the Apache Git Service.
To respond to
chia7712 merged PR #15645:
URL: https://github.com/apache/kafka/pull/15645
--
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:
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2082118117
@chia7712 Only 16 tests failing after CI rerun.
Please, take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub
chia7712 commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2081116897
@nizhikov could you please rebase code to trigger QA again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2080694465
@chia7712
`ConfigCommandIntegrationTest` passed on CI but many other tests failed.
Should we rerun or it's OK?
--
This is an automated message from the Apache Git Service.
To
nizhikov commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1581742421
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,291 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
chia7712 commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2080244534
@nizhikov I'm ok to accept this PR to be a java rewriting, and I file
https://issues.apache.org/jira/browse/KAFKA-16629 to add more tests in the
future. Hence, please fix comment
chia7712 commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1581621779
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,291 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2079970499
@chia7712 CI looks good to me. Do you have any other comments?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use
nizhikov commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1580728829
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,291 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
nizhikov commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1580711745
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,291 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
nizhikov commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1580711310
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,291 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
chia7712 commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1579206989
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,291 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
chia7712 commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2076742667
> Can you, please, take a look?
sure! will take a look later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2076735612
Hello @chia7712
1. Reworked test to use ClusterTestExtensions
2. Extend `testExitWithNonZeroStatusOnUpdatingUnallowedConfig` to check in
kraft mode.
3. Other two test zk
nizhikov commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1568506632
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
chia7712 commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1568266095
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2047311878
> I mean we can add more tests for zk and kraft in this PR, and it would be
nice to use new test infra (ClusterTestExtensions) to rewrite it. WDYT?
Agree. I will try to extend
chia7712 commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2047302613
> I thought we want to move all code related to the ConfigCommand to java
and remove it when ZK support will be dropped.
Personally, rewriting a class which will get removed
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2046644817
> I prefer to don't touch it. This class is all about zk and we can remove
it once we remove zk support.
So we want to rewrite in java KRaft tests only. Is it correct?
I
chia7712 commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2044427643
> Are you suggest to rewrite command and all tests to java but keep one test
in scala?
Let's rewrite all code to java.
Seems, like PR ready to review and merge :)
I prefer
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2044361200
> Do you have free cycles to take over it?
Yes, I do. But let's decide the order of the steps we want to make :)
--
This is an automated message from the Apache Git Service.
To
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2044356548
> Does ConfigCommandIntegrationTest have only zk-related tests?
Yes.
> If so, we don't need to rewrite it by java as it will be removed directly.
Are you suggest to
chia7712 commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2043536012
@nizhikov thanks for updated PR. I have a major question: Does
`ConfigCommandIntegrationTest` have only zk-related tests? If so, we don't need
to rewrite it by java as it will be
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2042116296
Hello @chia7712
> we can complete it in another PR before this PR.
`junit-platform.properties` added for core and tools modules.
Can you, please, review this test
nizhikov commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1553114805
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
nizhikov commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1553114426
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
nizhikov commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1553060780
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
chia7712 commented on code in PR #15645:
URL: https://github.com/apache/kafka/pull/15645#discussion_r1551656009
##
core/src/test/java/kafka/admin/ConfigCommandIntegrationTest.java:
##
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+
chia7712 commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2037116797
> Looks like CI OK. Can you, please, take a look?
thanks for updated PR. Sorry that I'm digging in the flaky tests in our CI,
but I will take a look at this one ASAP.
--
This
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2037008993
Hello @chia7712
Looks like CI OK. Can you, 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
nizhikov commented on PR #15645:
URL: https://github.com/apache/kafka/pull/15645#issuecomment-2031645630
Hello @chia7712 This is first PR that moves parts of ConfigCommand to java.
It contains rewritten `ConfigCommandIntegrationTest`.
Please, take a look.
--
This is an automated
nizhikov opened a new pull request, #15645:
URL: https://github.com/apache/kafka/pull/15645
This is first part of #15417 refactoring.
PR intention - split big PR in parts to simplify review.
PR contains `ConfigCommandIntegrationTest` rewritten in java
### Committer Checklist
33 matches
Mail list logo
