hachikuji commented on a change in pull request #10194: URL: https://github.com/apache/kafka/pull/10194#discussion_r583291501
########## File path: core/src/main/scala/kafka/server/ControllerApis.scala ########## @@ -251,45 +210,17 @@ class ControllerApis(val requestChannel: RequestChannel, // If this is considered to leak information about the broker version a workaround is to use SSL // with client authentication which is performed at an earlier stage of the connection where the // ApiVersionRequest is not available. - def createResponseCallback(features: FeatureMapAndEpoch, - requestThrottleMs: Int): ApiVersionsResponse = { + def createResponseCallback(requestThrottleMs: Int): ApiVersionsResponse = { val apiVersionRequest = request.body[ApiVersionsRequest] - if (apiVersionRequest.hasUnsupportedRequestVersion) + if (apiVersionRequest.hasUnsupportedRequestVersion) { apiVersionRequest.getErrorResponse(requestThrottleMs, Errors.UNSUPPORTED_VERSION.exception) - else if (!apiVersionRequest.isValid) + } else if (!apiVersionRequest.isValid) { apiVersionRequest.getErrorResponse(requestThrottleMs, Errors.INVALID_REQUEST.exception) - else { - val data = new ApiVersionsResponseData(). - setErrorCode(0.toShort). - setThrottleTimeMs(requestThrottleMs). - setFinalizedFeaturesEpoch(features.epoch()) - supportedFeatures.foreach { - case (k, v) => data.supportedFeatures().add(new SupportedFeatureKey(). - setName(k).setMaxVersion(v.max()).setMinVersion(v.min())) - } - // features.finalizedFeatures().asScala.foreach { - // case (k, v) => data.finalizedFeatures().add(new FinalizedFeatureKey(). - // setName(k).setMaxVersionLevel(v.max()).setMinVersionLevel(v.min())) - // } - ApiKeys.values().foreach { - key => - if (supportedApiKeys.contains(key)) { - data.apiKeys().add(new ApiVersion(). - setApiKey(key.id). - setMaxVersion(key.latestVersion()). - setMinVersion(key.oldestVersion())) - } - } - new ApiVersionsResponse(data) + } else { + apiVersionManager.apiVersionResponse(requestThrottleMs) Review comment: Right. I think the change makes sense in any case since the feature logic will end up getting handled by `ApiVersionManager` (as we do with the broker). ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org