You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "yashmayya (via GitHub)" <gi...@apache.org> on 2023/06/26 08:23:11 UTC

[GitHub] [kafka] yashmayya opened a new pull request, #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

yashmayya opened a new pull request, #13915:
URL: https://github.com/apache/kafka/pull/13915

   - https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect
   - https://issues.apache.org/jira/browse/KAFKA-14930
   - https://github.com/apache/kafka/pull/13465; https://github.com/apache/kafka/pull/13818
   - Previous documentation PRs for REST APIs introduced in KIP-875 - https://github.com/apache/kafka/pull/13587, https://github.com/apache/kafka/pull/13657
   
   - Preview of the documentation changes in this PR:
   <img width="1728" alt="Screenshot 2023-06-26 at 1 46 47 PM" src="https://github.com/apache/kafka/assets/23502577/ad594cf7-72f1-4984-bd95-8713a5e9105d">
   
   
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


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


[GitHub] [kafka] yashmayya commented on a diff in pull request #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

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


##########
docs/connect.html:
##########
@@ -313,7 +313,13 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management REST APIs (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state.</li>
+                <li><code>PATCH /connectors/{name}/offsets</code> - alter the offsets for a connector. The connector must exist and must be in the stopped state. The request body should be a JSON object containing a JSON array <code>offsets</code> field, similar to the response body of the <code>GET /connectors/{name}/offsets</code> REST API.</li>

Review Comment:
   The link to the generated OpenAPI docs are at the end of this section so I don't think it's necessary to add the same link here as well. Also, I think an actual example might be more helpful than the generated OpenAPI spec where the finest level of granularity is the `ConnectorOffset` schema describing the `partition` and `offset` keys having JSON object values. I was hoping that the link to the KIP should be sufficient, but I do see the value of including actual examples directly in the docs as well.



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


[GitHub] [kafka] yashmayya commented on a diff in pull request #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

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


##########
docs/connect.html:
##########
@@ -313,7 +313,13 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management REST APIs (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state.</li>
+                <li><code>PATCH /connectors/{name}/offsets</code> - alter the offsets for a connector. The connector must exist and must be in the stopped state. The request body should be a JSON object containing a JSON array <code>offsets</code> field, similar to the response body of the <code>GET /connectors/{name}/offsets</code> REST API.</li>

Review Comment:
   There is a link to the generated OpenAPI docs at the end of this section so I don't think it's necessary to add the same link here as well? Also, I think an actual example might be more helpful than the generated OpenAPI spec where the finest level of granularity is the [ConnectorOffset](https://github.com/apache/kafka/blob/c5889fceddb9a0174452ae60a57c8ff3f087a6a4/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/entities/ConnectorOffset.java#L41) schema describing the `partition` and `offset` keys having JSON object values. I was hoping that the link to the KIP should be sufficient, but I do see the value of including actual examples directly in the docs as well.



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


[GitHub] [kafka] yashmayya commented on pull request #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya commented on PR #13915:
URL: https://github.com/apache/kafka/pull/13915#issuecomment-1613311200

   Thanks Chris. I applied your suggestions and checked it out locally; things look good!


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


[GitHub] [kafka] C0urante merged pull request #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

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


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


[GitHub] [kafka] C0urante commented on a diff in pull request #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

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


##########
docs/connect.html:
##########
@@ -301,7 +301,7 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>GET /connectors/{name}/tasks</code> - get a list of tasks currently running for a connector</li>
         <li><code>GET /connectors/{name}/tasks/{taskid}/status</code> - get current status of the task, including if it is running, failed, paused, etc., which worker it is assigned to, and error information if it has failed</li>
         <li><code>PUT /connectors/{name}/pause</code> - pause the connector and its tasks, which stops message processing until the connector is resumed. Any resources claimed by its tasks are left allocated, which allows the connector to begin processing data quickly once it is resumed.</li>
-        <li><code>PUT /connectors/{name}/stop</code> - stop the connector and shut down its tasks, deallocating any resources claimed by its tasks. This is more efficient from a resource usage standpoint than pausing the connector, but can cause it to take longer to begin processing data once resumed.</li>
+        <li><code>PUT /connectors/{name}/stop</code> - stop the connector and shut down its tasks, deallocating any resources claimed by its tasks. This is more efficient from a resource usage standpoint than pausing the connector, but can cause it to take longer to begin processing data once resumed. Note that the offsets for a connector can be only modified via the offsets management endpoints if it is in the stopped state</li>

Review Comment:
   ```suggestion
           <li id="connect_stopconnector"><code>PUT /connectors/{name}/stop</code> - stop the connector and shut down its tasks, deallocating any resources claimed by its tasks. This is more efficient from a resource usage standpoint than pausing the connector, but can cause it to take longer to begin processing data once resumed. Note that the offsets for a connector can be only modified via the offsets management endpoints if it is in the stopped state</li>
   ```



##########
docs/connect.html:
##########
@@ -313,7 +313,22 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management endpoints (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state</li>
+                <li><code>PATCH /connectors/{name}/offsets</code> - alter the offsets for a connector. The connector must exist and must be in the stopped state. The request body should be a JSON object containing a JSON array <code>offsets</code> field, similar to the response body of the <code>GET /connectors/{name}/offsets</code> endpoint</li>

Review Comment:
   ```suggestion
                   <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state (see <a href="#connect_stopconnector"><code>PUT /connectors/{name}/stop</code></a>)</li>
                   <li><code>PATCH /connectors/{name}/offsets</code> - alter the offsets for a connector. The connector must exist and must be in the stopped state (see <a href="#connect_stopconnector"><code>PUT /connectors/{name}/stop</code></a>). The request body should be a JSON object containing a JSON array <code>offsets</code> field, similar to the response body of the <code>GET /connectors/{name}/offsets</code> endpoint</li>
   ```



##########
docs/connect.html:
##########
@@ -313,7 +313,13 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management REST APIs (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state.</li>
+                <li><code>PATCH /connectors/{name}/offsets</code> - alter the offsets for a connector. The connector must exist and must be in the stopped state. The request body should be a JSON object containing a JSON array <code>offsets</code> field, similar to the response body of the <code>GET /connectors/{name}/offsets</code> REST API.</li>

Review Comment:
   The example is great, thanks 👍



##########
docs/connect.html:
##########
@@ -313,7 +313,22 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management endpoints (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state</li>
+                <li><code>PATCH /connectors/{name}/offsets</code> - alter the offsets for a connector. The connector must exist and must be in the stopped state. The request body should be a JSON object containing a JSON array <code>offsets</code> field, similar to the response body of the <code>GET /connectors/{name}/offsets</code> endpoint</li>
+                An example request body for the <code>FileStreamSourceConnector</code>:
+                <pre class="line-numbers"><code class="json">
+{"offsets": [{"partition": {"filename": "test.txt"}, "offset": {"position": 30}}]}

Review Comment:
   We can make this prettier:
   ```suggestion
   {
     "offsets": [
       {
         "partition": {
           "filename": "test.txt"
         },
         "offset": {
           "position": 30
         }
       }
     ]
   }
   ```



##########
docs/connect.html:
##########
@@ -313,7 +313,22 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management endpoints (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state</li>
+                <li><code>PATCH /connectors/{name}/offsets</code> - alter the offsets for a connector. The connector must exist and must be in the stopped state. The request body should be a JSON object containing a JSON array <code>offsets</code> field, similar to the response body of the <code>GET /connectors/{name}/offsets</code> endpoint</li>
+                An example request body for the <code>FileStreamSourceConnector</code>:
+                <pre class="line-numbers"><code class="json">
+{"offsets": [{"partition": {"filename": "test.txt"}, "offset": {"position": 30}}]}
+                </code></pre>
+                An example request body for the <code>FileStreamSinkConnector</code>:
+                <pre class="line-numbers"><code class="json">
+{"offsets": [{"partition": {"kafka_topic": "test", "kafka_partition": 0}, "offset": {"kafka_offset": 5}}, {"partition": {"kafka_topic": "test", "kafka_partition": 1}, "offset": null}]}

Review Comment:
   We can make this prettier:
   ```suggestion
   {
     "offsets": [
       {
         "partition": {
           "kafka_topic": "test",
           "kafka_partition": 0
         },
         "offset": {
           "kafka_offset": 5
         }
       },
       {
         "partition": {
           "kafka_topic": "test",
           "kafka_partition": 1
         },
         "offset": null
       }
     ]
   }
   ```



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


[GitHub] [kafka] C0urante commented on pull request #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

Posted by "C0urante (via GitHub)" <gi...@apache.org>.
C0urante commented on PR #13915:
URL: https://github.com/apache/kafka/pull/13915#issuecomment-1613382253

   Given that this change only touches on our HTML docs, I'll merge without waiting for CI.


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


[GitHub] [kafka] yashmayya commented on a diff in pull request #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

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


##########
docs/connect.html:
##########
@@ -313,7 +313,13 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management REST APIs (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state.</li>

Review Comment:
   Hm the `PUT /connectors/{name}/stop` endpoint docs are in the same `connect_rest` section as these docs and I don't think it is possible to link to individual list items inside the section?



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


[GitHub] [kafka] C0urante commented on a diff in pull request #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

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


##########
docs/connect.html:
##########
@@ -301,7 +301,7 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>GET /connectors/{name}/tasks</code> - get a list of tasks currently running for a connector</li>
         <li><code>GET /connectors/{name}/tasks/{taskid}/status</code> - get current status of the task, including if it is running, failed, paused, etc., which worker it is assigned to, and error information if it has failed</li>
         <li><code>PUT /connectors/{name}/pause</code> - pause the connector and its tasks, which stops message processing until the connector is resumed. Any resources claimed by its tasks are left allocated, which allows the connector to begin processing data quickly once it is resumed.</li>
-        <li><code>PUT /connectors/{name}/stop</code> - stop the connector and shut down its tasks, deallocating any resources claimed by its tasks. This is more efficient from a resource usage standpoint than pausing the connector, but can cause it to take longer to begin processing data once resumed.</li>
+        <li><code>PUT /connectors/{name}/stop</code> - stop the connector and shut down its tasks, deallocating any resources claimed by its tasks. This is more efficient from a resource usage standpoint than pausing the connector, but can cause it to take longer to begin processing data once resumed. Note that the offsets for a connector can be modified via the offsets management REST APIs only if it is in the stopped state.</li>

Review Comment:
   ```suggestion
           <li><code>PUT /connectors/{name}/stop</code> - stop the connector and shut down its tasks, deallocating any resources claimed by its tasks. This is more efficient from a resource usage standpoint than pausing the connector, but can cause it to take longer to begin processing data once resumed. Note that the offsets for a connector can be only modified via the offsets management endpoints if it is in the stopped state.</li>
   ```



##########
docs/connect.html:
##########
@@ -313,7 +313,13 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management REST APIs (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state.</li>

Review Comment:
   Nit: for consistency, we should leave out the period from the last sentence of each item
   ```suggestion
                   <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state</li>
   ```
   
   Also, would it be possible to link to the docs for the `PUT /connectors/{name}/stop` endpoint when we refer to it here?



##########
docs/connect.html:
##########
@@ -313,7 +313,13 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management REST APIs (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state.</li>
+                <li><code>PATCH /connectors/{name}/offsets</code> - alter the offsets for a connector. The connector must exist and must be in the stopped state. The request body should be a JSON object containing a JSON array <code>offsets</code> field, similar to the response body of the <code>GET /connectors/{name}/offsets</code> REST API.</li>

Review Comment:
   Same nit RE trailing period, and same question about linking to the docs for the `PUT /connectors/{name}/stop` endpoint:
   ```suggestion
                   <li><code>PATCH /connectors/{name}/offsets</code> - alter the offsets for a connector. The connector must exist and must be in the stopped state. The request body should be a JSON object containing a JSON array <code>offsets</code> field, similar to the response body of the <code>GET /connectors/{name}/offsets</code> endpoint</li>
   ```



##########
docs/connect.html:
##########
@@ -313,7 +313,13 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management REST APIs (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state.</li>
+                <li><code>PATCH /connectors/{name}/offsets</code> - alter the offsets for a connector. The connector must exist and must be in the stopped state. The request body should be a JSON object containing a JSON array <code>offsets</code> field, similar to the response body of the <code>GET /connectors/{name}/offsets</code> REST API.</li>

Review Comment:
   Also, do you think we can include either a link to the generated OpenAPI docs for this endpoint, or a fleshed-out example of a request for this endpoint?
   
   I like that we're calling out the symmetry between the request format for this endpoint and the response format for the `GET /connectors/{name}/offsets` endpoint, but I suspect it may be easier on users if we also provide examples for altering the offsets of a sink and source connector (including some null partitions to denote resets).
   
   Possibly creeping out of scope for this PR, but we might have an additional docs section that goes into detail on managing offsets with connectors (including example requests/responses) and then link to that section from each of these list items, instead of trying to put all of the information for these endpoints into this section. This is definitely not a blocker, though.



##########
docs/connect.html:
##########
@@ -313,7 +313,13 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management REST APIs (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):

Review Comment:
   Nit:
   ```suggestion
           <li>Offsets management endpoints (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
   ```



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


[GitHub] [kafka] C0urante commented on a diff in pull request #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

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


##########
docs/connect.html:
##########
@@ -301,7 +301,7 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>GET /connectors/{name}/tasks</code> - get a list of tasks currently running for a connector</li>
         <li><code>GET /connectors/{name}/tasks/{taskid}/status</code> - get current status of the task, including if it is running, failed, paused, etc., which worker it is assigned to, and error information if it has failed</li>
         <li><code>PUT /connectors/{name}/pause</code> - pause the connector and its tasks, which stops message processing until the connector is resumed. Any resources claimed by its tasks are left allocated, which allows the connector to begin processing data quickly once it is resumed.</li>
-        <li><code>PUT /connectors/{name}/stop</code> - stop the connector and shut down its tasks, deallocating any resources claimed by its tasks. This is more efficient from a resource usage standpoint than pausing the connector, but can cause it to take longer to begin processing data once resumed.</li>
+        <li><code>PUT /connectors/{name}/stop</code> - stop the connector and shut down its tasks, deallocating any resources claimed by its tasks. This is more efficient from a resource usage standpoint than pausing the connector, but can cause it to take longer to begin processing data once resumed. Note that the offsets for a connector can be modified via the offsets management REST APIs only if it is in the stopped state.</li>

Review Comment:
   ```suggestion
           <li><code>PUT /connectors/{name}/stop</code> - stop the connector and shut down its tasks, deallocating any resources claimed by its tasks. This is more efficient from a resource usage standpoint than pausing the connector, but can cause it to take longer to begin processing data once resumed. Note that the offsets for a connector can be only modified via the offsets management endpoints if it is in the stopped state</li>
   ```



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


[GitHub] [kafka] C0urante commented on a diff in pull request #13915: KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect

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


##########
docs/connect.html:
##########
@@ -313,7 +313,13 @@ <h4><a id="connect_rest" href="#connect_rest">REST API</a></h4>
         <li><code>DELETE /connectors/{name}</code> - delete a connector, halting all tasks and deleting its configuration</li>
         <li><code>GET /connectors/{name}/topics</code> - get the set of topics that a specific connector is using since the connector was created or since a request to reset its set of active topics was issued</li>
         <li><code>PUT /connectors/{name}/topics/reset</code> - send a request to empty the set of active topics of a connector</li>
-        <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details)</li>
+        <li>Offsets management REST APIs (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
+            <ul>
+                <li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
+                <li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state.</li>

Review Comment:
   It's possible; I'll add some GH suggestions demonstrating how.



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