jolshan commented on code in PR #15685:
URL: https://github.com/apache/kafka/pull/15685#discussion_r1612319940
##########
core/src/main/scala/kafka/tools/StorageTool.scala:
##########
@@ -109,6 +111,52 @@ object StorageTool extends Logging {
}
}
+ private def validateMetadataVersion(metadataVersion: MetadataVersion,
config: Option[KafkaConfig]): Unit = {
+ if (!metadataVersion.isKRaftSupported) {
+ throw new TerseFailure(s"Must specify a valid KRaft metadata.version of
at least ${MetadataVersion.IBP_3_0_IV0}.")
+ }
+ if (!metadataVersion.isProduction) {
+ if (config.get.unstableMetadataVersionsEnabled) {
+ System.out.println(s"WARNING: using pre-production metadata.version
$metadataVersion.")
+ } else {
+ throw new TerseFailure(s"The metadata.version $metadataVersion is not
ready for production use yet.")
+ }
+ }
+ }
+
+ private[tools] def generateFeatureRecords(metadataRecords:
ArrayBuffer[ApiMessageAndVersion],
+ metadataVersion: MetadataVersion,
+ specifiedFeatures: Map[String,
java.lang.Short],
+ allFeatures: List[Features],
+ usesVersionDefault: Boolean): Unit
= {
+ // If we are using --version-default, the default is based on the metadata
version.
+ val metadataVersionForDefault = if (usesVersionDefault)
Optional.of(metadataVersion) else Optional.empty[MetadataVersion]()
Review Comment:
We do not. This was changed yesterday and the code was as you say in the
comment. Here is the behavior as it stands now:
> Do we get the same result by passing in that metadataVersion to
feature.defaultValue()
If we follow the protocol of creating a new MV for each new feature and
making them production ready at the same time then the answer to your question
is yes. If we want to codify this and require a new MV (used only for mapping a
default) for every new feature to be created when we mark the feature as
production ready, I can switch it back to how it was yesterday.
##########
core/src/main/scala/kafka/tools/StorageTool.scala:
##########
@@ -109,6 +111,52 @@ object StorageTool extends Logging {
}
}
+ private def validateMetadataVersion(metadataVersion: MetadataVersion,
config: Option[KafkaConfig]): Unit = {
+ if (!metadataVersion.isKRaftSupported) {
+ throw new TerseFailure(s"Must specify a valid KRaft metadata.version of
at least ${MetadataVersion.IBP_3_0_IV0}.")
+ }
+ if (!metadataVersion.isProduction) {
+ if (config.get.unstableMetadataVersionsEnabled) {
+ System.out.println(s"WARNING: using pre-production metadata.version
$metadataVersion.")
+ } else {
+ throw new TerseFailure(s"The metadata.version $metadataVersion is not
ready for production use yet.")
+ }
+ }
+ }
+
+ private[tools] def generateFeatureRecords(metadataRecords:
ArrayBuffer[ApiMessageAndVersion],
+ metadataVersion: MetadataVersion,
+ specifiedFeatures: Map[String,
java.lang.Short],
+ allFeatures: List[Features],
+ usesVersionDefault: Boolean): Unit
= {
+ // If we are using --version-default, the default is based on the metadata
version.
+ val metadataVersionForDefault = if (usesVersionDefault)
Optional.of(metadataVersion) else Optional.empty[MetadataVersion]()
Review Comment:
We do not. This was changed yesterday and the code was as you say in the
comment.
> Do we get the same result by passing in that metadataVersion to
feature.defaultValue()
If we follow the protocol of creating a new MV for each new feature and
making them production ready at the same time then the answer to your question
is yes. If we want to codify this and require a new MV (used only for mapping a
default) for every new feature to be created when we mark the feature as
production ready, I can switch it back to how it was yesterday.
--
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
