cmccabe commented on a change in pull request #10194: URL: https://github.com/apache/kafka/pull/10194#discussion_r583294273
########## 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: agreed. we will have to have some way for the controller to update those fields in ApiVersionsManager (possibly an atomic / volatile field or something). ---------------------------------------------------------------- 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