[GitHub] [kafka] ijuma commented on a change in pull request #9748: MINOR: Simplify ApiKeys by relying on ApiMessageType and removing unused methods

[GitBox]

ijuma commented on a change in pull request #9748:
URL: https://github.com/apache/kafka/pull/9748#discussion_r543472687



##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java
##########
@@ -289,49 +129,40 @@ public Struct parseResponse(short version, ByteBuffer 
buffer) {
     /** indicates whether the API is enabled for forwarding **/
     public final boolean forwardable;
 
-    public final Schema[] requestSchemas;
-    public final Schema[] responseSchemas;
     public final boolean requiresDelayedAllocation;
 
-    ApiKeys(int id, String name, Schema[] requestSchemas, Schema[] 
responseSchemas) {
-        this(id, name, false, requestSchemas, responseSchemas);
-    }
+    public final ApiMessageType messageType;
 
-    ApiKeys(int id, String name, boolean clusterAction, Schema[] 
requestSchemas, Schema[] responseSchemas) {
-        this(id, name, clusterAction, RecordBatch.MAGIC_VALUE_V0, 
requestSchemas, responseSchemas);
+    ApiKeys(ApiMessageType messageType) {
+        this(messageType, false);
     }
 
-    ApiKeys(int id, String name, Schema[] requestSchemas, Schema[] 
responseSchemas, boolean forwardable) {
-        this(id, name, false, RecordBatch.MAGIC_VALUE_V0, true, 
requestSchemas, responseSchemas, forwardable);
+    ApiKeys(ApiMessageType messageType, boolean clusterAction) {
+        this(messageType, clusterAction, RecordBatch.MAGIC_VALUE_V0, false);
     }
 
-    ApiKeys(int id, String name, boolean clusterAction, boolean isEnabled, 
Schema[] requestSchemas, Schema[] responseSchemas) {
-        this(id, name, clusterAction, RecordBatch.MAGIC_VALUE_V0, isEnabled, 
requestSchemas, responseSchemas, false);
+    ApiKeys(ApiMessageType messageType, boolean clusterAction, boolean 
forwardable) {
+        this(messageType, clusterAction, RecordBatch.MAGIC_VALUE_V0, 
forwardable);
     }
 
-    ApiKeys(int id, String name, boolean clusterAction, byte 
minRequiredInterBrokerMagic,
-            Schema[] requestSchemas, Schema[] responseSchemas) {
-        this(id, name, clusterAction, minRequiredInterBrokerMagic, true, 
requestSchemas, responseSchemas, false);
+    ApiKeys(ApiMessageType messageType, boolean clusterAction, byte 
minRequiredInterBrokerMagic, boolean forwardable) {
+        this(messageType, clusterAction, minRequiredInterBrokerMagic, 
forwardable, true);
     }
 
     ApiKeys(
-        int id,
-        String name,
+        ApiMessageType messageType,
         boolean clusterAction,
         byte minRequiredInterBrokerMagic,
-        boolean isEnabled,
-        Schema[] requestSchemas,
-        Schema[] responseSchemas,
-        boolean forwardable
+        boolean forwardable,
+        boolean isEnabled
     ) {
+        short id = messageType.apiKey();

Review comment:
       I took a look at this and not clear how to validate this in the 
generator. I added unit tests to `ApiMessageTypeTest` instead and I think 
that's good enough for now. What do you think?




----------------------------------------------------------------
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