You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "chirag-wadhwa5 (via GitHub)" <gi...@apache.org> on 2025/02/17 08:47:48 UTC

[PR] KAFKA-16718 [1/n]: Added DeleteShareGroupOffsets request and response schema [kafka]

chirag-wadhwa5 opened a new pull request, #18927:
URL: https://github.com/apache/kafka/pull/18927

   This PR is the first in a set to include the delete share group offsets functionality. This PR adds the request and response schemas for DeleteShareGroupOffsets. 
   
   Reference: [JIRA](https://issues.apache.org/jira/browse/KAFKA-16718)


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-16718 [1/n]: Added DeleteShareGroupOffsets request and response schema [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18927:
URL: https://github.com/apache/kafka/pull/18927#discussion_r1959486260


##########
docs/security.html:
##########
@@ -2291,6 +2291,14 @@ <h5 class="anchor-heading"><a id="operations_resources_and_protocols" class="anc
             <td>Topic</td>
             <td></td>
         </tr>
+        <tr>

Review Comment:
   I think this needs to be 2 table rows. I suggest:
   ```
           <tr>
               <td>DELETE_SHARE_GROUP_OFFSETS (92)</td>
               <td>Delete</td>
               <td>Group</td>
               <td>To delete the offset information for a partition in a share group, the application must have privileges on the
                   group and the topic. Group access is checked first, then topic access.</td>
           </tr>
           <tr>
               <td>DELETE_SHARE_GROUP_OFFSETS (92)</td>
               <td>Read</td>
               <td>Topic</td>
               <td></td>
           </tr>
   
   ```



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-16718 [1/n]: Added DeleteShareGroupOffsets request and response schema [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield merged PR #18927:
URL: https://github.com/apache/kafka/pull/18927


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-16718 [1/n]: Added DeleteShareGroupOffsets request and response schema [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18927:
URL: https://github.com/apache/kafka/pull/18927#discussion_r1959374984


##########
server/src/main/java/org/apache/kafka/network/RequestConvertToJson.java:
##########
@@ -623,6 +629,8 @@ public static JsonNode response(AbstractResponse response, short version) {
                 return DeleteGroupsResponseDataJsonConverter.write(((DeleteGroupsResponse) response).data(), version);
             case DELETE_RECORDS:
                 return DeleteRecordsResponseDataJsonConverter.write(((DeleteRecordsResponse) response).data(), version);
+            case DELETE_SHARE_GROUP_OFFSETS:

Review Comment:
   Please can you move `ALTER_SHARE_GROUP_OFFSETS` in this file (2 instances) to their correct places too? Thanks.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-16718 [1/n]: Added DeleteShareGroupOffsets request and response schema [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18927:
URL: https://github.com/apache/kafka/pull/18927#discussion_r1958205103


##########
clients/src/main/resources/common/message/DeleteShareGroupOffsetsRequest.json:
##########
@@ -0,0 +1,32 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+{
+  "apiKey": 92,
+  "type": "request",
+  "listeners": ["broker"],
+  "name": "DeleteShareGroupOffsetsRequest",
+  "validVersions": "0",
+  "flexibleVersions": "0+",
+  "fields": [
+    { "name": "GroupId", "type": "string", "versions": "0+", "entityType": "groupId",
+      "about": "The group identifier." },
+    { "name": "Topics", "type": "[]DeleteShareGroupOffsetsRequestTopic", "versions": "0+",
+      "about": "The topics to delete offsets for.",  "fields": [
+      { "name": "TopicName", "type": "string", "versions": "0+", "entityType": "topicName",
+        "about": "The topic name." }

Review Comment:
   As discussed, because each partition has its own deletion error code, we should adjust both request and response schemas to include partitions.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-16718 [1/n]: Added DeleteShareGroupOffsets request and response schema [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18927:
URL: https://github.com/apache/kafka/pull/18927#discussion_r1958430956


##########
server/src/main/java/org/apache/kafka/network/RequestConvertToJson.java:
##########
@@ -465,6 +469,8 @@ public static JsonNode request(AbstractRequest request) {
                 return DescribeShareGroupOffsetsRequestDataJsonConverter.write(((DescribeShareGroupOffsetsRequest) request).data(), request.version());
             case ALTER_SHARE_GROUP_OFFSETS:
                 return AlterShareGroupOffsetsRequestDataJsonConverter.write(((AlterShareGroupOffsetsRequest) request).data(), request.version());
+            case DELETE_SHARE_GROUP_OFFSETS:

Review Comment:
   The cases in this switch statement are intended to be in alphabetical order. Please can you move `ALTER_SHARE_GROUP_OFFSETS` and `DELETE_SHARE_GROUP_OFFSETS` in to their correct places in the order. Thanks.



##########
clients/src/main/resources/common/message/DeleteShareGroupOffsetsResponse.json:
##########
@@ -0,0 +1,51 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+{
+  "apiKey": 92,
+  "type": "response",
+  "name": "DeleteShareGroupOffsetsResponse",
+  "validVersions": "0",
+  "flexibleVersions": "0+",
+  // Supported errors:
+  // - GROUP_AUTHORIZATION_FAILED (version 0+)
+  // - NOT_COORDINATOR (version 0+)
+  // - COORDINATOR_NOT_AVAILABLE (version 0+)
+  // - COORDINATOR_LOAD_IN_PROGRESS (version 0+)
+  // - GROUP_ID_NOT_FOUND (version 0+)
+  // - NON_EMPTY_GROUP (version 0+)
+  // - KAFKA_STORAGE_ERROR (version 0+)
+  // - INVALID_REQUEST (version 0+)
+  // - UNKNOWN_SERVER_ERROR (version 0+)
+  "fields": [
+    { "name": "ThrottleTimeMs", "type": "int32", "versions": "0+",
+      "about": "The duration in milliseconds for which the request was throttled due to a quota violation, or zero if the request did not violate any quota." },
+    { "name": "Responses", "type": "[]DeleteShareGroupOffsetsResponseTopic", "versions": "0+",

Review Comment:
   There needs to be a top-level error code/message too.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-16718 [1/n]: Added DeleteShareGroupOffsets request and response schema [kafka]

Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #18927:
URL: https://github.com/apache/kafka/pull/18927#discussion_r1959369758


##########
docs/security.html:
##########
@@ -2277,6 +2277,12 @@ <h5 class="anchor-heading"><a id="operations_resources_and_protocols" class="anc
             <td>Group</td>
             <td></td>
         </tr>
+        <tr>
+            <td>DELETE_SHARE_GROUP_OFFSETS (92)</td>
+            <td>Alter</td>

Review Comment:
   The required authorizations are DELETE GROUP and READ TOPIC.



##########
clients/src/main/resources/common/message/DeleteShareGroupOffsetsResponse.json:
##########
@@ -0,0 +1,55 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+{
+  "apiKey": 92,
+  "type": "response",
+  "name": "DeleteShareGroupOffsetsResponse",
+  "validVersions": "0",
+  "flexibleVersions": "0+",
+  // Supported errors:
+  // - GROUP_AUTHORIZATION_FAILED (version 0+)
+  // - NOT_COORDINATOR (version 0+)

Review Comment:
   There is `TOPIC_AUTHORIZATION_FAILED` too.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org