Re: [PR] Image test improvements [kafka]
ahuang98 commented on code in PR #15373:
URL: https://github.com/apache/kafka/pull/15373#discussion_r1544932474
##
metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java:
##
@@ -88,7 +101,7 @@ public class ClusterImageTest {
setId(2).
setEpoch(123).
setIncarnationId(Uuid.fromString("hr4TVh3YQiu3p16Awkka6w")).
-setListeners(Arrays.asList(new Endpoint("PLAINTEXT",
SecurityProtocol.PLAINTEXT, "localhost", 9093))).
+setListeners(Arrays.asList(new Endpoint("PLAINTEXT",
SecurityProtocol.PLAINTEXT, "localhost", 9094))).
Review Comment:
Responded on https://issues.apache.org/jira/browse/KAFKA-16447, someone is
volunteering to fix, will check in again tomorrow!
--
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] Image test improvements [kafka]
ahuang98 commented on code in PR #15373:
URL: https://github.com/apache/kafka/pull/15373#discussion_r1544930494
##
metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java:
##
@@ -88,7 +101,7 @@ public class ClusterImageTest {
setId(2).
setEpoch(123).
setIncarnationId(Uuid.fromString("hr4TVh3YQiu3p16Awkka6w")).
-setListeners(Arrays.asList(new Endpoint("PLAINTEXT",
SecurityProtocol.PLAINTEXT, "localhost", 9093))).
+setListeners(Arrays.asList(new Endpoint("PLAINTEXT",
SecurityProtocol.PLAINTEXT, "localhost", 9094))).
Review Comment:
Thanks folks, will open up a fix soon
--
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] Image test improvements [kafka]
jolshan commented on code in PR #15373:
URL: https://github.com/apache/kafka/pull/15373#discussion_r1544929534
##
metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java:
##
@@ -88,7 +101,7 @@ public class ClusterImageTest {
setId(2).
setEpoch(123).
setIncarnationId(Uuid.fromString("hr4TVh3YQiu3p16Awkka6w")).
-setListeners(Arrays.asList(new Endpoint("PLAINTEXT",
SecurityProtocol.PLAINTEXT, "localhost", 9093))).
+setListeners(Arrays.asList(new Endpoint("PLAINTEXT",
SecurityProtocol.PLAINTEXT, "localhost", 9094))).
Review Comment:
I just found this as well. I opened
https://issues.apache.org/jira/browse/KAFKA-16451 but will close as a duplicate
--
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] Image test improvements [kafka]
chia7712 commented on code in PR #15373:
URL: https://github.com/apache/kafka/pull/15373#discussion_r1544339792
##
metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java:
##
@@ -88,7 +101,7 @@ public class ClusterImageTest {
setId(2).
setEpoch(123).
setIncarnationId(Uuid.fromString("hr4TVh3YQiu3p16Awkka6w")).
-setListeners(Arrays.asList(new Endpoint("PLAINTEXT",
SecurityProtocol.PLAINTEXT, "localhost", 9093))).
+setListeners(Arrays.asList(new Endpoint("PLAINTEXT",
SecurityProtocol.PLAINTEXT, "localhost", 9094))).
Review Comment:
I have filed a ticket https://issues.apache.org/jira/browse/KAFKA-16447
please feel free to take over it
--
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] Image test improvements [kafka]
chia7712 commented on code in PR #15373:
URL: https://github.com/apache/kafka/pull/15373#discussion_r1544335647
##
metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java:
##
@@ -88,7 +101,7 @@ public class ClusterImageTest {
setId(2).
setEpoch(123).
setIncarnationId(Uuid.fromString("hr4TVh3YQiu3p16Awkka6w")).
-setListeners(Arrays.asList(new Endpoint("PLAINTEXT",
SecurityProtocol.PLAINTEXT, "localhost", 9093))).
+setListeners(Arrays.asList(new Endpoint("PLAINTEXT",
SecurityProtocol.PLAINTEXT, "localhost", 9094))).
Review Comment:
`ReplicaManagerTest` reuse the `IMAGE1` to test the listeners, hence this
change break some test cases. Do you have free cycle to fix it? If not, I can
file PR to fix it :)
--
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] Image test improvements [kafka]
mimaison merged PR #15373: URL: https://github.com/apache/kafka/pull/15373 -- 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] Image test improvements [kafka]
mimaison commented on code in PR #15373:
URL: https://github.com/apache/kafka/pull/15373#discussion_r1539799687
##
metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java:
##
@@ -186,6 +264,19 @@ public void testImage2RoundTrip() {
testToImage(IMAGE2);
}
+public void testApplyDelta2() {
Review Comment:
Sorry I should have added that after enabling that test, it is failing.
Expected:
`brokers([...], 1(BrokerRegistration(id=1, epoch=1001,
incarnationId=U52uRe20RsGI0RvpcTx33Q,
listeners=[Endpoint(listenerName='PLAINTEXT', securityProtocol=PLAINTEXT,
host='localhost', port=9093)], supportedFeatures={foo: 1-3},
rack=Optional.empty, fenced=true, inControlledShutdown=true,
isMigratingZkBroker=false, directories=[])), [...]`
Actual:
`brokers([...], 1(BrokerRegistration(id=1, epoch=1001,
incarnationId=U52uRe20RsGI0RvpcTx33Q,
listeners=[Endpoint(listenerName='PLAINTEXT', securityProtocol=PLAINTEXT,
host='localhost', port=9093)], supportedFeatures={foo: 1-3},
rack=Optional.empty, fenced=false, inControlledShutdown=false,
isMigratingZkBroker=false, directories=[])), [...]`
Broker 1 has `fenced=false` and `inControlledShutdown=false` while we
expected both to be `true`.
--
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] Image test improvements [kafka]
mimaison commented on code in PR #15373:
URL: https://github.com/apache/kafka/pull/15373#discussion_r1539417806
##
metadata/src/test/java/org/apache/kafka/image/ClusterImageTest.java:
##
@@ -186,6 +264,19 @@ public void testImage2RoundTrip() {
testToImage(IMAGE2);
}
+public void testApplyDelta2() {
Review Comment:
Is this missing the `@Test` annotation?
##
metadata/src/test/java/org/apache/kafka/image/ImageDowngradeTest.java:
##
@@ -128,6 +128,39 @@ public void testPreControlledShutdownStateVersion() {
TEST_RECORDS.get(1)));
}
+/**
+ * Test downgrading to a MetadataVersion that doesn't support ZK migration.
+ */
+@Test
+public void testPreZkMigrationSupportVersion() throws Throwable {
Review Comment:
This does not throw so we can remove `throws Throwable`
--
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] Image test improvements [kafka]
mimaison commented on PR #15373: URL: https://github.com/apache/kafka/pull/15373#issuecomment-2012481246 @ahuang98 The [last CI](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15373/3/#showFailuresLink) run for this PR had a lot of test failures. I think if you rebase on trunk that should clear most 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
[PR] Image test improvements [kafka]
ahuang98 opened a new pull request, #15373: URL: https://github.com/apache/kafka/pull/15373 More commit history and comments from https://github.com/apache/kafka/pull/14206 ### 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
