m1a2st commented on code in PR #18845:
URL: https://github.com/apache/kafka/pull/18845#discussion_r1950727327
##########
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##########
@@ -44,25 +44,6 @@
*/
Review Comment:
I think we should also update the java doc, replace old example like`
IBP_0_10_0-IV0` to `IBP_3_3_IV3`
##########
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##########
@@ -368,7 +320,8 @@ public static MetadataVersion fromVersionString(String
versionString) {
key = String.join(".", Arrays.copyOfRange(versionSegments, 0,
numSegments));
}
return Optional.ofNullable(IBP_VERSIONS.get(key)).orElseThrow(() ->
- new IllegalArgumentException("Version " + versionString + " is not
a valid version")
+ new IllegalArgumentException("Version " + versionString + " is not
a valid version. The minimum version is " + MINIMUM_VERSION
+ + " and the maximum version is " + latestTesting())
Review Comment:
This method is used in production code in `StorageTool.scala` and others,
maybe we should not expose `latestTesting()` version, should use
`latestProduction()`.
##########
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##########
@@ -378,7 +331,8 @@ public static MetadataVersion fromFeatureLevel(short
version) {
return metadataVersion;
}
}
- throw new IllegalArgumentException("No MetadataVersion with feature
level " + version);
+ throw new IllegalArgumentException("No MetadataVersion with feature
level " + version + ". Valid feature levels are from "
+ + MINIMUM_VERSION.featureLevel + " to " +
latestTesting().featureLevel + ".");
Review Comment:
ditto, we should use latestProduction().
##########
tools/src/test/java/org/apache/kafka/tools/FeatureCommandTest.java:
##########
@@ -250,66 +250,77 @@ public void testHandleDescribe() {
}
});
assertEquals(format("Feature: foo.bar\tSupportedMinVersion:
0\tSupportedMaxVersion: 10\tFinalizedVersionLevel: 5\tEpoch: 123%n" +
- "Feature: metadata.version\tSupportedMinVersion:
3.3-IV0\tSupportedMaxVersion: 3.3-IV3\tFinalizedVersionLevel: 3.3-IV2\tEpoch:
123"), describeResult);
+ "Feature: metadata.version\tSupportedMinVersion:
3.3-IV3\tSupportedMaxVersion: 3.5-IV0\tFinalizedVersionLevel: 3.4-IV0\tEpoch:
123"), describeResult);
}
@Test
- public void testHandleUpgrade() {
+ public void testHandleUpgradeToUnsupportedMetadataVersion() {
Map<String, Object> namespace = new HashMap<>();
namespace.put("metadata", "3.3-IV1");
Review Comment:
nit: This `3.3-IV1` variable also can add to `MetadataVersionTestUtils`
class.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
