junrao commented on code in PR #20185:
URL: https://github.com/apache/kafka/pull/20185#discussion_r2223756688
##########
core/src/main/scala/kafka/tools/StorageTool.scala:
##########
@@ -128,9 +128,21 @@ object StorageTool extends Logging {
setIgnoreFormatted(namespace.getBoolean("ignore_formatted")).
setControllerListenerName(config.controllerListenerNames.get(0)).
setMetadataLogDirectory(config.metadataLogDir)
- Option(namespace.getString("release_version")).foreach(
- releaseVersion => formatter.
- setReleaseVersion(MetadataVersion.fromVersionString(releaseVersion)))
+
+ def metadataVersionsToString(first: MetadataVersion, last:
MetadataVersion): String = {
+ val versions = MetadataVersion.VERSIONS.slice(first.ordinal,
last.ordinal + 1)
+ versions.map(_.toString).mkString(", ")
+ }
+ Option(namespace.getString("release_version")).foreach(releaseVersion => {
+ try {
+
formatter.setReleaseVersion(MetadataVersion.fromVersionString(releaseVersion))
+ } catch {
+ case _: Throwable =>
+ throw new TerseFailure(s"Unknown metadata.version $releaseVersion.
Supported metadata.version are " +
+ s"${metadataVersionsToString(MetadataVersion.MINIMUM_VERSION,
MetadataVersion.latestProduction())}")
Review Comment:
This is not very accurate. If the internal config
unstable.feature.versions.enable is set, we actually allow non-production MVs.
All callers of `MetadataVersion.fromVersionString` are either on the server
side or from the tools and should have access to the
unstable.feature.versions.enable. Perhaps it's better to change
`MetadataVersion.fromVersionString` to take an enableUnstableFeature flag.
--
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
