showuon commented on PR #15663:
URL: https://github.com/apache/kafka/pull/15663#issuecomment-2040993296
LGTM! Thanks @FrankYang0529 ! Really nice catch!
--
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
chia7712 merged PR #15663:
URL: https://github.com/apache/kafka/pull/15663
--
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:
chia7712 commented on PR #15663:
URL: https://github.com/apache/kafka/pull/15663#issuecomment-2040889410
I'm going to merge this PR in order to make other tool tests can run with
KRaft. We can address all late reviews in other PRs.
--
This is an automated message from the Apache Git
chia7712 commented on PR #15663:
URL: https://github.com/apache/kafka/pull/15663#issuecomment-2040402625
@ijuma Could you please take a look at this PR?
--
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
FrankYang0529 commented on PR #15663:
URL: https://github.com/apache/kafka/pull/15663#issuecomment-2039753742
> > I think the class file size increasing is indeed a direct drawback after
adding -parameter option because we'll include all the parameters into .class
files. I'd like to know
chia7712 commented on PR #15663:
URL: https://github.com/apache/kafka/pull/15663#issuecomment-2039097741
> I think the class file size increasing is indeed a direct drawback after
adding -parameter option because we'll include all the parameters into .class
files. I'd like to know if
showuon commented on PR #15663:
URL: https://github.com/apache/kafka/pull/15663#issuecomment-2039081866
Sorry @FrankYang0529 , I saw this:
https://stackoverflow.com/questions/44067477/drawbacks-of-javac-parameters-flag
> Briefly, the stated reasons to make parameter names optional are
FrankYang0529 commented on code in PR #15663:
URL: https://github.com/apache/kafka/pull/15663#discussion_r1553003195
##
build.gradle:
##
@@ -270,6 +270,7 @@ subprojects {
options.compilerArgs << "-Xlint:-serial"
options.compilerArgs << "-Xlint:-try"
FrankYang0529 commented on PR #15663:
URL: https://github.com/apache/kafka/pull/15663#issuecomment-2039041551
> > However, this way still doesn't check whether "parameter name" is
correct. Probably, we can give another check is that if display name contains
zk or kraft, but not quorum,
chia7712 commented on code in PR #15663:
URL: https://github.com/apache/kafka/pull/15663#discussion_r1552981401
##
build.gradle:
##
@@ -270,6 +270,7 @@ subprojects {
options.compilerArgs << "-Xlint:-serial"
options.compilerArgs << "-Xlint:-try"
chia7712 commented on PR #15663:
URL: https://github.com/apache/kafka/pull/15663#issuecomment-2039025377
> However, this way still doesn't check whether "parameter name" is correct.
Probably, we can give another check is that if display name contains zk or
kraft, but not quorum, then
FrankYang0529 commented on PR #15663:
URL: https://github.com/apache/kafka/pull/15663#issuecomment-2039019880
> 2. How could we avoid this things happen in the future? Like adding some
checking before the tests startup or something? Do you have any idea?
I don't have a good idea to
showuon commented on PR #15663:
URL: https://github.com/apache/kafka/pull/15663#issuecomment-2038807461
@FrankYang0529 , thanks for the fix! Nice catch!
Questions:
1. How could we confirm the `String quorum` missing only in
`DeleteOffsetsConsumerGroupCommandIntegrationTest`, not
FrankYang0529 opened a new pull request, #15663:
URL: https://github.com/apache/kafka/pull/15663
Following test cases don't really run kraft case. The reason is that the
test info doesn't contain parameter name, so it always returns false in
TestInfoUtils#isKRaft.
-
14 matches
Mail list logo
