Re: [PR] KAFKA-15949: Unify metadata.version format in log and error message [kafka]
showuon merged PR #15505: URL: https://github.com/apache/kafka/pull/15505 -- 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-15949: Unify metadata.version format in log and error message [kafka]
showuon commented on PR #15505: URL: https://github.com/apache/kafka/pull/15505#issuecomment-202025 Failed tests are 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 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-15949: Unify metadata.version format in log and error message [kafka]
showuon commented on PR #15505: URL: https://github.com/apache/kafka/pull/15505#issuecomment-2019263725 Re-running CI build: https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15505/6/ -- 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-15949: Unify metadata.version format in log and error message [kafka]
FrankYang0529 commented on PR #15505: URL: https://github.com/apache/kafka/pull/15505#issuecomment-2018399116 > @FrankYang0529 , there are some tests failing because of your change. Please take a look. Thanks. > > https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15505/4 Sorry for that. I will double check the test result next time. I have fixed error in `testScramWithBadMetadataVersion`, `testMetadataVersionChangeExceptionToString`, and `testCreateClusterInvalidMetadataVersion`. Thank you. -- 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-15949: Unify metadata.version format in log and error message [kafka]
showuon commented on PR #15505: URL: https://github.com/apache/kafka/pull/15505#issuecomment-2017562723 @FrankYang0529 , there are some tests failing because of your change. Please take a look. Thanks. https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15505/4 -- 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-15949: Unify metadata.version format in log and error message [kafka]
showuon commented on PR #15505: URL: https://github.com/apache/kafka/pull/15505#issuecomment-2011051818 Retriggering the CI build: https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-15505/4/ -- 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-15949: Unify metadata.version format in log and error message [kafka]
FrankYang0529 commented on code in PR #15505:
URL: https://github.com/apache/kafka/pull/15505#discussion_r1531943054
##
core/src/main/scala/kafka/tools/StorageTool.scala:
##
@@ -139,7 +139,7 @@ object StorageTool extends Logging {
action(storeTrue())
formatParser.addArgument("--release-version", "-r").
action(store()).
- help(s"A KRaft release version to use for the initial metadata version.
The minimum is 3.0, the default is
${MetadataVersion.LATEST_PRODUCTION.version()}")
+ help(s"A KRaft release version to use for the initial metadata.version.
The minimum is ${MetadataVersion.IBP_3_0_IV0}, the default is
${MetadataVersion.LATEST_PRODUCTION.version()}")
Review Comment:
Yes, it looks like we only use `.version()` in few messages, so I remove it.
Thank you.
--
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-15949: Unify metadata.version format in log and error message [kafka]
showuon commented on code in PR #15505:
URL: https://github.com/apache/kafka/pull/15505#discussion_r1527973511
##
core/src/main/scala/kafka/tools/StorageTool.scala:
##
@@ -139,7 +139,7 @@ object StorageTool extends Logging {
action(storeTrue())
formatParser.addArgument("--release-version", "-r").
action(store()).
- help(s"A KRaft release version to use for the initial metadata version.
The minimum is 3.0, the default is
${MetadataVersion.LATEST_PRODUCTION.version()}")
+ help(s"A KRaft release version to use for the initial metadata.version.
The minimum is ${MetadataVersion.IBP_3_0_IV0}, the default is
${MetadataVersion.LATEST_PRODUCTION.version()}")
Review Comment:
OK, thanks for the explanation. Could we at least make them consistency (all
with `.version()` or all without 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] KAFKA-15949: Unify metadata.version format in log and error message [kafka]
FrankYang0529 commented on code in PR #15505:
URL: https://github.com/apache/kafka/pull/15505#discussion_r1521640919
##
core/src/main/scala/kafka/tools/StorageTool.scala:
##
@@ -139,7 +139,7 @@ object StorageTool extends Logging {
action(storeTrue())
formatParser.addArgument("--release-version", "-r").
action(store()).
- help(s"A KRaft release version to use for the initial metadata version.
The minimum is 3.0, the default is
${MetadataVersion.LATEST_PRODUCTION.version()}")
+ help(s"A KRaft release version to use for the initial metadata.version.
The minimum is ${MetadataVersion.IBP_3_0_IV0}, the default is
${MetadataVersion.LATEST_PRODUCTION.version()}")
Review Comment:
Hi @showuon, thanks for the review. Using `MetadataVersion.IBP_3_0_IV0` or
`MetadataVersion.IBP_3_0_IV0.version()` has same result. Both `toString` and
`version` functions return `ibpVersion`:
https://github.com/apache/kafka/blob/58ddd693e69599b177d09c2e384f31e7f5e11171/server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java#L523-L525
https://github.com/apache/kafka/blob/58ddd693e69599b177d09c2e384f31e7f5e11171/server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java#L632-L634
--
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-15949: Unify metadata.version format in log and error message [kafka]
showuon commented on code in PR #15505:
URL: https://github.com/apache/kafka/pull/15505#discussion_r1520960222
##
core/src/main/scala/kafka/tools/StorageTool.scala:
##
@@ -139,7 +139,7 @@ object StorageTool extends Logging {
action(storeTrue())
formatParser.addArgument("--release-version", "-r").
action(store()).
- help(s"A KRaft release version to use for the initial metadata version.
The minimum is 3.0, the default is
${MetadataVersion.LATEST_PRODUCTION.version()}")
+ help(s"A KRaft release version to use for the initial metadata.version.
The minimum is ${MetadataVersion.IBP_3_0_IV0}, the default is
${MetadataVersion.LATEST_PRODUCTION.version()}")
Review Comment:
I saw 2 formats: `MetadataVersion.IBP_3_0_IV0` and
`MetadataVersion.IBP_3_0_IV0.version()` here. Could you help me understand what
output it will be for each of them? And I think we should make them consistent.
Same as other places.
--
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] KAFKA-15949: Unify metadata.version format in log and error message [kafka]
FrankYang0529 opened a new pull request, #15505: URL: https://github.com/apache/kafka/pull/15505 There were different words for `metadata.version` like `metadata version` or `metadataVersion`. Unify format as `metadata.version`. ### 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
