You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/06/18 17:54:25 UTC

[GitHub] [kafka] vamossagar12 opened a new pull request, #12306: KAFKA-13976: improved open-api specs initial commit'

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

   *More detailed description of your change,
   if necessary. The PR title and PR message become
   the squashed commit message, so use a separate
   comment to ping reviewers.*
   
   *Summary of testing strategy (including rationale)
   for the feature or bug fix. Unit and/or integration
   tests are expected for any behaviour change and
   system tests should be considered for larger changes.*
   
   ### 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] vamossagar12 commented on pull request #12306: KAFKA-13976: Improvements for OpenAPI specs

Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on PR #12306:
URL: https://github.com/apache/kafka/pull/12306#issuecomment-1213341834

   @tombentley / @mimaison Plz review this PR whenever you get the chance..


-- 
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] vamossagar12 commented on pull request #12306: KAFKA-13976: Improvements for OpenAPI specs

Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on PR #12306:
URL: https://github.com/apache/kafka/pull/12306#issuecomment-1164645606

   @tombentley i saw the original PR for adding OpenAPI spec was reviewed by you as well. Plz review this one as well whenever you get the chance. 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


[GitHub] [kafka] tombentley commented on a diff in pull request #12306: KAFKA-13976: Improvements for OpenAPI specs

Posted by GitBox <gi...@apache.org>.
tombentley commented on code in PR #12306:
URL: https://github.com/apache/kafka/pull/12306#discussion_r908280202


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java:
##########
@@ -105,7 +115,28 @@ private <T> void addConnectorPlugins(Collection<PluginDesc<T>> plugins, Collecti
 
     @PUT
     @Path("/{pluginName}/config/validate")
-    @Operation(summary = "Validate the provided configuration against the configuration definition for the specified pluginName")
+    @Operation(
+            summary = "Validate the provided configuration against the configuration definition for the specified pluginName",
+            parameters = {
+                    @Parameter(in = ParameterIn.PATH, schema = @Schema(implementation = StringSchema.class), name = "pluginName"),

Review Comment:
   `StringSchema` doesn't get rendered as a `type: string` as I'd expect, but rather as a `StringSchema` schema object with `type: object` and a whole bunch of schema properties. I think `@Schema(type = "string")` will give you what you need.



-- 
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-13976: Improvements for OpenAPI specs [kafka]

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

   @mimaison , yes sorry this completely missed my radar. Let me get to it some time next week.


-- 
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] vamossagar12 commented on pull request #12306: KAFKA-13976: Improvements for OpenAPI specs

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

   Converting to draft. Need some time before I get to this.


-- 
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] vamossagar12 commented on pull request #12306: KAFKA-13976: Improvements for OpenAPI specs

Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on PR #12306:
URL: https://github.com/apache/kafka/pull/12306#issuecomment-1217789901

   > Thanks for the PR. Quite a few things are still not quite right, see my comments.
   
   Thanks @mimaison . Sorry about that. I see there are quite a few misses in the PR. Would correct them as you pointed. 


-- 
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-13976: Improvements for OpenAPI specs [kafka]

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

   @mimaison , turns out I mayn't be able to spend time on this one. I will close this PR and unassigned the JIRA. 


-- 
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] vamossagar12 commented on pull request #12306: KAFKA-13976: Improvements for OpenAPI specs

Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on PR #12306:
URL: https://github.com/apache/kafka/pull/12306#issuecomment-1172892866

   Thanks for the review @tombentley . I made the changes. 


-- 
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] vamossagar12 commented on a diff in pull request #12306: KAFKA-13976: Improvements for OpenAPI specs

Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on code in PR #12306:
URL: https://github.com/apache/kafka/pull/12306#discussion_r912359623


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java:
##########
@@ -105,7 +115,28 @@ private <T> void addConnectorPlugins(Collection<PluginDesc<T>> plugins, Collecti
 
     @PUT
     @Path("/{pluginName}/config/validate")
-    @Operation(summary = "Validate the provided configuration against the configuration definition for the specified pluginName")
+    @Operation(
+            summary = "Validate the provided configuration against the configuration definition for the specified pluginName",
+            parameters = {

Review Comment:
   oh that's a good catch. I missed that.. Thanks for pointing it out. I removed the parameters from the operation annotation wherever the query param isn't hidden.



-- 
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-13976: Improvements for OpenAPI specs [kafka]

Posted by "vamossagar12 (via GitHub)" <gi...@apache.org>.
vamossagar12 closed pull request #12306: KAFKA-13976: Improvements for OpenAPI specs
URL: https://github.com/apache/kafka/pull/12306


-- 
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] vamossagar12 commented on pull request #12306: KAFKA-13976: Improvements for OpenAPI specs

Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on PR #12306:
URL: https://github.com/apache/kafka/pull/12306#issuecomment-1196564092

   @tombentley could you plz take a look whenever you get the chance. 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


[GitHub] [kafka] vamossagar12 commented on pull request #12306: KAFKA-13976: improved open-api specs initial commit'

Posted by GitBox <gi...@apache.org>.
vamossagar12 commented on PR #12306:
URL: https://github.com/apache/kafka/pull/12306#issuecomment-1159524714

   @mimaison First cut of the PR.. I haven't replaced Response classes as yet. Also, I am new to OpenAPI and there are some `Map` responses in the resources. Currently, started using MapSchecma.class but i am not sure if it would work properly or not.


-- 
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] mimaison commented on a diff in pull request #12306: KAFKA-13976: Improvements for OpenAPI specs

Posted by GitBox <gi...@apache.org>.
mimaison commented on code in PR #12306:
URL: https://github.com/apache/kafka/pull/12306#discussion_r946792078


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java:
##########
@@ -112,7 +119,25 @@ public void requestTimeout(long requestTimeoutMs) {
 
     @PUT
     @Path("/{pluginName}/config/validate")
-    @Operation(summary = "Validate the provided configuration against the configuration definition for the specified pluginName")
+    @Operation(
+            summary = "Validate the provided configuration against the configuration definition for the specified pluginName",
+            requestBody = @RequestBody(
+                    required = true,
+                    content = @Content(
+                            mediaType = "application/json",
+                            schema = @Schema(implementation = MapSchema.class),

Review Comment:
   This generates a ton of lines in the spec file which I'm not sure totally make sense. Have you tried using the spec file to generate code?
   
   Same for all the other places where you put `MapSchema`.



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -107,7 +117,23 @@ public void requestTimeout(long requestTimeoutMs) {
 
     @GET
     @Path("/")
-    @Operation(summary = "List all active connectors")
+    @Operation(
+            summary = "List all active connectors",
+            parameters = @Parameter(

Review Comment:
   Could we document this query parameter like we've done for `connectorsOnly`? See https://github.com/apache/kafka/blob/trunk/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java#L148



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -179,7 +237,27 @@ public ConnectorInfo getConnector(final @PathParam("connector") String connector
 
     @GET
     @Path("/{connector}/config")
-    @Operation(summary = "Get the configuration for the specified connector")
+    @Operation(
+            summary = "Get the configuration for the specified connector",
+            parameters = {
+                    @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class), name = "forward")
+            }
+    )
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "200",
+                    description = "Connector config details retrieved successfully",

Review Comment:
   Can we use `Connector configuration retrieved successfully`?



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -145,7 +171,26 @@ public Response listConnectors(
 
     @POST
     @Path("/")
-    @Operation(summary = "Create a new connector")
+    @Operation(
+            summary = "Create a new connector",
+            requestBody = @RequestBody(
+                    content = @Content(mediaType = "application/json", schema = @Schema(implementation = CreateConnectorRequest.class)),
+                    required = true,
+                    description = "Name and configuration parameters of the new connector. All values in config should be strings."
+            ),
+            parameters = @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class))

Review Comment:
   Do we need this? I'm assuming it's about the `forward` query parameter but this does not even include the name! Also `forward` is already documented below.



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -260,7 +417,42 @@ public Response putConnectorConfig(final @PathParam("connector") String connecto
 
     @POST
     @Path("/{connector}/restart")
-    @Operation(summary = "Restart the specified connector")
+    @Operation(
+            summary = "Restart the specified connector",
+            parameters = {
+                    @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class), name = "forward")
+            }
+    )
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "200",
+                    description = "Named connector exists and the server has successfully stopped and begun restarting only the Connector object"
+            ),
+            @ApiResponse(
+                    responseCode = "202",
+                    description = "Named connector exists and the server has successfully and durably recorded the request to stop and " +
+                            "begin restarting at least one failed or running Connector object and Task instances",
+                    content = @Content(mediaType = "application/json", schema = @Schema(implementation = ConnectorStateInfo.class))
+            ),
+            @ApiResponse(
+                    responseCode = "204",
+                    description = "Operation succeeded, but there is no content in the response"
+            ),
+            @ApiResponse(
+                    responseCode = "404",
+                    description = "Named connector does not exist"
+            ),
+            @ApiResponse(
+                    responseCode = "409",
+                    description = "A rebalance is needed, forthcoming, or underway while restarting any of the Connector and/or Task objects"

Review Comment:
   We should use original descriptions rather than copying from the Confluent docs!



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -237,7 +371,30 @@ public Response resetConnectorActiveTopics(final @PathParam("connector") String
 
     @PUT
     @Path("/{connector}/config")
-    @Operation(summary = "Create or reconfigure the specified connector")
+    @Operation(
+            summary = "Create or reconfigure the specified connector",
+            parameters = {
+                    @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class), name = "forward")
+            },
+            requestBody = @RequestBody(
+                    content = @Content(
+                            mediaType = "application/json",
+                            schema = @Schema(implementation = MapSchema.class),
+                            additionalPropertiesSchema = @Schema(type = "string")
+                    )
+            )
+    )
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "201",

Review Comment:
   Is it 201 or 200?



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -345,6 +565,14 @@ public void fenceZombies(final @PathParam("connector") String connector,
     @GET
     @Path("/{connector}/tasks/{task}/status")
     @Operation(summary = "Get the state of the specified task for the specified connector")
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "200",
+                    content = @Content(mediaType = "application/json", array = @ArraySchema(schema = @Schema(implementation = ConnectorStateInfo.TaskState.class))),
+                    description = "Task status fetched successfully for specified connector"

Review Comment:
   We've used `retrieved` everywhere so let's stick to that instead of introducing `fetched` here. We can also drop the `for specified connector` ending.



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -260,7 +417,42 @@ public Response putConnectorConfig(final @PathParam("connector") String connecto
 
     @POST
     @Path("/{connector}/restart")
-    @Operation(summary = "Restart the specified connector")
+    @Operation(
+            summary = "Restart the specified connector",
+            parameters = {
+                    @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class), name = "forward")
+            }
+    )
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "200",
+                    description = "Named connector exists and the server has successfully stopped and begun restarting only the Connector object"
+            ),
+            @ApiResponse(
+                    responseCode = "202",
+                    description = "Named connector exists and the server has successfully and durably recorded the request to stop and " +
+                            "begin restarting at least one failed or running Connector object and Task instances",
+                    content = @Content(mediaType = "application/json", schema = @Schema(implementation = ConnectorStateInfo.class))
+            ),
+            @ApiResponse(
+                    responseCode = "204",
+                    description = "Operation succeeded, but there is no content in the response"
+            ),
+            @ApiResponse(
+                    responseCode = "404",
+                    description = "Named connector does not exist"
+            ),
+            @ApiResponse(
+                    responseCode = "409",
+                    description = "A rebalance is needed, forthcoming, or underway while restarting any of the Connector and/or Task objects"

Review Comment:
   If other descriptions have been copied, let's update them too



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -319,7 +531,11 @@ public List<TaskInfo> getTaskConfigs(final @PathParam("connector") String connec
 
     @POST
     @Path("/{connector}/tasks")
-    @Operation(hidden = true, summary = "This operation is only for inter-worker communications")
+    @Operation(

Review Comment:
   We don't need to document the request body since it's not a public API. Let's undo this change. 



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -188,9 +266,30 @@ public Map<String, String> getConnectorConfig(final @PathParam("connector") Stri
         return completeOrForwardRequest(cb, "/connectors/" + connector + "/config", "GET", headers, null, forward);
     }
 
+

Review Comment:
   We don't need this extra line



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -366,7 +600,19 @@ public void restartTask(final @PathParam("connector") String connector,
 
     @DELETE
     @Path("/{connector}")
-    @Operation(summary = "Delete the specified connector")
+    @Operation(
+            summary = "Delete the specified connector",
+            parameters = {
+                    @Parameter(in = ParameterIn.QUERY, schema = @Schema(implementation = BooleanSchema.class), name = "forward", hidden = true)
+            }
+    )
+    @ApiResponses(value = {
+            @ApiResponse(responseCode = "204", description = "Connector deleted successfully"),
+            @ApiResponse(
+                    responseCode = "409",
+                    description = "Error while deleting new connector. Either rabalance is in progress or the connector already exists"

Review Comment:
   `or the connector already exists` does not seem quite right!



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -237,7 +371,30 @@ public Response resetConnectorActiveTopics(final @PathParam("connector") String
 
     @PUT
     @Path("/{connector}/config")
-    @Operation(summary = "Create or reconfigure the specified connector")
+    @Operation(
+            summary = "Create or reconfigure the specified connector",
+            parameters = {
+                    @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class), name = "forward")
+            },
+            requestBody = @RequestBody(
+                    content = @Content(
+                            mediaType = "application/json",
+                            schema = @Schema(implementation = MapSchema.class),
+                            additionalPropertiesSchema = @Schema(type = "string")
+                    )
+            )
+    )
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "201",
+                    description = "New connector created successfully",

Review Comment:
   What about `Connector created or updated successfully`?



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -188,9 +266,30 @@ public Map<String, String> getConnectorConfig(final @PathParam("connector") Stri
         return completeOrForwardRequest(cb, "/connectors/" + connector + "/config", "GET", headers, null, forward);
     }
 
+
     @GET
     @Path("/{connector}/tasks-config")
-    @Operation(summary = "Get the configuration of all tasks for the specified connector")
+    @Operation(
+            summary = "Get the configuration of all tasks for the specified connector",
+            parameters = {
+                    @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class), name = "forward")
+            }
+    )
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "200",
+                    description = "Connector tasks config details retrieved successfully",

Review Comment:
   What about `Connector tasks configuration retrieved successfully`?



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -237,7 +371,30 @@ public Response resetConnectorActiveTopics(final @PathParam("connector") String
 
     @PUT
     @Path("/{connector}/config")
-    @Operation(summary = "Create or reconfigure the specified connector")
+    @Operation(
+            summary = "Create or reconfigure the specified connector",
+            parameters = {
+                    @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class), name = "forward")
+            },
+            requestBody = @RequestBody(
+                    content = @Content(
+                            mediaType = "application/json",
+                            schema = @Schema(implementation = MapSchema.class),
+                            additionalPropertiesSchema = @Schema(type = "string")
+                    )
+            )
+    )
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "201",
+                    description = "New connector created successfully",
+                    content = @Content(mediaType = "application/json", schema = @Schema(implementation = ConnectorInfo.class))
+            ),
+            @ApiResponse(
+                    responseCode = "409",
+                    description = "Error while creating new connector. Either rabalance is in progress or the connector already exists"

Review Comment:
   This message is not correct. Let's use `Error while creating or updating connector. A rebalance is in progress`.



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -299,15 +494,32 @@ public Response pauseConnector(@PathParam("connector") String connector, final @
     @PUT
     @Path("/{connector}/resume")
     @Operation(summary = "Resume the specified connector",
-               description = "This operation is idempotent and has no effects if the connector is already running")
+            description = "This operation is idempotent and has no effects if the connector is already running")
+    @ApiResponses(value = {
+            @ApiResponse(responseCode = "202", description = "Connector resume request accepted"),
+            @ApiResponse(responseCode = "404", description = "Named connector doesn't exist")
+    })
     public Response resumeConnector(@PathParam("connector") String connector) {
         herder.resumeConnector(connector);
         return Response.accepted().build();
     }
 
     @GET
     @Path("/{connector}/tasks")
-    @Operation(summary = "List all tasks for the specified connector")
+    @Operation(
+            summary = "List all tasks for the specified connector",
+            parameters = {
+                    @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class), name = "forward")
+            }
+    )
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "200",
+                    content = @Content(mediaType = "application/json", array = @ArraySchema(schema = @Schema(implementation = TaskInfo.class))),
+                    description = "Connector resume request accepted"

Review Comment:
   That's not the right description



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -145,7 +171,26 @@ public Response listConnectors(
 
     @POST
     @Path("/")
-    @Operation(summary = "Create a new connector")
+    @Operation(
+            summary = "Create a new connector",
+            requestBody = @RequestBody(
+                    content = @Content(mediaType = "application/json", schema = @Schema(implementation = CreateConnectorRequest.class)),
+                    required = true,
+                    description = "Name and configuration parameters of the new connector. All values in config should be strings."
+            ),
+            parameters = @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class))

Review Comment:
   Same a few more times below



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -222,6 +346,16 @@ public Response getConnectorActiveTopics(final @PathParam("connector") String co
     @PUT
     @Path("/{connector}/topics/reset")
     @Operation(summary = "Reset the list of topics actively used by the specified connector")
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "200",
+                    description = "Connector status retrieved successfully",

Review Comment:
   Again not the right description



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -202,14 +301,39 @@ public Map<ConnectorTaskId, Map<String, String>> getTasksConfig(
 
     @GET
     @Path("/{connector}/status")
-    @Operation(summary = "Get the status for the specified connector")
+    @Operation(
+            summary = "Get the status for the specified connector"
+    )
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "200",
+                    description = "Connector status retrieved successfully",
+                    content = @Content(mediaType = "application/json", schema = @Schema(implementation = ConnectorStateInfo.class))
+            ),
+            @ApiResponse(responseCode = "404", description = "Connector not found")
+    })
     public ConnectorStateInfo getConnectorStatus(final @PathParam("connector") String connector) {
         return herder.connectorStatus(connector);
     }
 
     @GET
     @Path("/{connector}/topics")
     @Operation(summary = "Get the list of topics actively used by the specified connector")
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "200",
+                    description = "Connector status retrieved successfully",

Review Comment:
   It's not the right description



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -145,7 +171,26 @@ public Response listConnectors(
 
     @POST
     @Path("/")
-    @Operation(summary = "Create a new connector")
+    @Operation(
+            summary = "Create a new connector",
+            requestBody = @RequestBody(
+                    content = @Content(mediaType = "application/json", schema = @Schema(implementation = CreateConnectorRequest.class)),
+                    required = true,
+                    description = "Name and configuration parameters of the new connector. All values in config should be strings."
+            ),
+            parameters = @Parameter(in = ParameterIn.QUERY, hidden = true, schema = @Schema(implementation = BooleanSchema.class))
+    )
+    @ApiResponses(value = {
+            @ApiResponse(
+                    responseCode = "201",
+                    description = "New connector created successfully",
+                    content = @Content(mediaType = "application/json", schema = @Schema(implementation = ConnectorInfo.class))
+            ),
+            @ApiResponse(
+                    responseCode = "409",
+                    description = "Error while creating new connector. Either rabalance is in progress or the connector already exists"

Review Comment:
   Typo in `rebalance`. Same a few more times below.
   Also can we rephrase it to: `Either a rebalance is in progress or the connector already exists`?



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java:
##########
@@ -332,7 +548,11 @@ public void putTaskConfigs(final @PathParam("connector") String connector,
 
     @PUT
     @Path("/{connector}/fence")
-    @Operation(hidden = true, summary = "This operation is only for inter-worker communications")
+    @Operation(

Review Comment:
   Not a public API, let's undo too



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/LoggingResource.java:
##########
@@ -128,7 +146,17 @@ public Response getLogger(final @PathParam("logger") String namedLogger) {
      */
     @PUT
     @Path("/{logger}")
-    @Operation(summary = "Set the level for the specified logger")
+    @Operation(
+            summary = "Set the level for the specified logger",
+            requestBody = @RequestBody(
+                    required = true,
+                    content = @Content(
+                            mediaType = "application/json",
+                            schema = @Schema(implementation = MapSchema.class),

Review Comment:
   Should we cover the return type too? Currently it generates
   ```
   responses:
           default:
             content:
               application/json: {}
             description: default response
   ```
   which is not great



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java:
##########
@@ -112,7 +119,25 @@ public void requestTimeout(long requestTimeoutMs) {
 
     @PUT
     @Path("/{pluginName}/config/validate")
-    @Operation(summary = "Validate the provided configuration against the configuration definition for the specified pluginName")
+    @Operation(
+            summary = "Validate the provided configuration against the configuration definition for the specified pluginName",
+            requestBody = @RequestBody(
+                    required = true,
+                    content = @Content(
+                            mediaType = "application/json",
+                            schema = @Schema(implementation = MapSchema.class),
+                            additionalPropertiesSchema = @Schema(type = "string")
+                    )
+            )
+    )
+    @ApiResponses(
+            value = {
+                    @ApiResponse(
+                            responseCode = "201",

Review Comment:
   Isn't it returning 200 instead?



-- 
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-13976: Improvements for OpenAPI specs [kafka]

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

   @vamossagar12 Are you still planning to complete this item?


-- 
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] tombentley commented on a diff in pull request #12306: KAFKA-13976: Improvements for OpenAPI specs

Posted by GitBox <gi...@apache.org>.
tombentley commented on code in PR #12306:
URL: https://github.com/apache/kafka/pull/12306#discussion_r908272740


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java:
##########
@@ -105,7 +115,28 @@ private <T> void addConnectorPlugins(Collection<PluginDesc<T>> plugins, Collecti
 
     @PUT
     @Path("/{pluginName}/config/validate")
-    @Operation(summary = "Validate the provided configuration against the configuration definition for the specified pluginName")
+    @Operation(
+            summary = "Validate the provided configuration against the configuration definition for the specified pluginName",
+            parameters = {

Review Comment:
   It looks like declaring parameters like this results in duplicates in the output, for example:
   
   ```yaml
     /connector-plugins/{pluginName}/config/validate:
       put:
         operationId: validateConfigs
         parameters:
         - in: path
           name: pluginName
           required: true
           schema:
             $ref: '#/components/schemas/StringSchema'
         - in: path
           name: pluginName
           required: true
           schema:
             type: string
   ```



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