rhauch commented on a change in pull request #11132:
URL: https://github.com/apache/kafka/pull/11132#discussion_r676987061
##########
File path:
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java
##########
@@ -269,7 +269,7 @@ public Response restartConnector(final
@PathParam("connector") String connector,
FutureCallback<Void> cb = new FutureCallback<>();
herder.restartConnector(connector, cb);
completeOrForwardRequest(cb, forwardingPath, "POST", headers,
null, forward);
- return Response.ok().build();
+ return Response.noContent().build();
Review comment:
This returns `204 No Content`, right? But doesn't the old code (prior to
this feature) return `200 OK` with no content?
[KIP-745](https://cwiki.apache.org/confluence/display/KAFKA/KIP-745%3A+Connect+API+to+restart+connector+and+tasks)
mentions using `202 Accepted` for the new behavior, but keeps the `200 OK`
behavior when `includeTasks=false` and `onlyFailed=false`.
Are you suggesting that we can't return `200 OK` here if there is no
response body and should therefore update the KIP, or can this line be left as
is and we change the `RestClient` to properly handle a null content? The issue
for this test failure ([KAFKA-111]()) mentions [this
code](https://github.com/apache/kafka/blob/d89ea74918ce0a1f2309a09473c9500c52af3b10/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/RestClient.java#L134-L136)
in the `RestClient`, which handles all 200 to <300 status codes the same way,
so could we just improve that?
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
