junrao commented on code in PR #17128:
URL: https://github.com/apache/kafka/pull/17128#discussion_r1751071116
##########
clients/src/main/java/org/apache/kafka/common/requests/ApiVersionsResponse.java:
##########
@@ -289,19 +289,17 @@ private static SupportedFeatureKeyCollection
maybeFilterSupportedFeatureKeys(
SupportedFeatureKeyCollection converted = new
SupportedFeatureKeyCollection();
for (Map.Entry<String, SupportedVersionRange> feature :
latestSupportedFeatures.features().entrySet()) {
final SupportedVersionRange versionRange = feature.getValue();
- final SupportedFeatureKey key = new SupportedFeatureKey();
- key.setName(feature.getKey());
if (alterV0 && versionRange.min() == 0) {
// Some older clients will have deserialization problems if a
feature's
// minimum supported level is 0. Therefore, when preparing
ApiVersionResponse
- // at versions less than 4, we must set the minimum version
for these features
- // to 1 rather than 0. See KAFKA-17011 for details.
- key.setMinVersion((short) 1);
+ // at versions less than 4, we must omit these features. See
KAFKA-17492.
Review Comment:
We need to change the comment like the following in ApiVersionResponse.json
and BrokerRegistrationRequest.json.
`Note: in v0-v3, features with MinSupportedVersion = 0 show up with
MinSupportedVersion = 1.
`
##########
core/src/test/scala/unit/kafka/server/ApiVersionsResponseIntegrationTest.scala:
##########
@@ -72,4 +72,12 @@ class ApiVersionsResponseIntegrationTest extends
BaseRequestTest {
assertEquals(name, key.name())
assertEquals(expectedMinVersion, key.minVersion())
}
+
+ def assertFeatureMissing(
Review Comment:
Could this be private?
--
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]
