-
Notifications
You must be signed in to change notification settings - Fork 14.9k
KAFKA-13780: Generate OpenAPI file for Connect REST API #12067
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
970cd12 to
5ad0a92
Compare
5ad0a92 to
2cfd143
Compare
New gradle task: connect:runtime:genConnectOpenAPIDocs that generates connect_rest.yaml under docs/generated This task is executed when siteDocsTar runs.
2cfd143 to
5364ebe
Compare
|
@kkonstantine @rhauch Can you take a look? |
tombentley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mimaison!
- Do you have any plans for generating some HTML from the YAML?
- What about documenting the expected non-200 response codes?
|
|
||
| @POST | ||
| @Path("/{connector}/restart") | ||
| @Operation(summary = "Restart the specified connector") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention includeTasks and onlyFailed parameters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added descriptions for these parameters
|
|
||
| @PUT | ||
| @Path("/{connector}/pause") | ||
| @Operation(summary = "Pause the specified connector") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mention the semantics of pausing an already paused connector.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think nothing happens when pausing an already paused connector. Isn't it the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right, but then we should say that it's idempotent.
|
|
||
| @PUT | ||
| @Path("/{connector}/resume") | ||
| @Operation(summary = "Resume the specified connector") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, we should explain what happens if you resume a non-paused connector.
|
|
||
| @GET | ||
| @Path("/") | ||
| @Operation(summary = "Get details about the Connect cluster") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not just about the cluster, it's also info about this particular server.
| @Operation(summary = "Get details about the Connect cluster") | |
| @Operation(summary = "Get details about this Connect worker and the id of the Kafka cluster it is connected to") |
| expand(kafkaVersion: "$rootProject.version") | ||
| } | ||
|
|
||
| task genConnectOpenAPIDocs(type: io.swagger.v3.plugins.gradle.tasks.ResolveTask, dependsOn: setVersionInOpenAPISpec) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also depends on having the connect runtime classes/jar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this happens automatically because of classpath = sourceSets.main.runtimeClasspath
| @GET | ||
| @Path("/") | ||
| @Operation(summary = "List the current loggers that have their levels explicitly set and their log levels") | ||
| public Response listLoggers() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Response return types in this and other Resource classes means there are no schemas generated, which seems less than ideal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is not great. I think this should be fixable without requiring a KIP, but let's do this in a separate PR
|
|
||
| @GET | ||
| @Path("/") | ||
| @Operation(summary = "List all connector plugins installed") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should explain the behaivour of the connectorsOnly parameter too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
Thanks @tombentley for taking a look!
I did take a look at generating HTML when working on this weeks ago but I did not find any generators that I really liked. I tried the htnl, html2 and dynamic-html generators from https://openapi-generator.tech/docs/generators#documentation-generators but found they were either buggy or required quite a bit of template customization to be nicely readable. So for now I'm sticking to only generating the spec file.
Yes definitively something we need to do. I'm tempted to do it in a separate PR where we'll also use types that describe the response schemas. |
tombentley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modulo the idempotency comment this LGTM. +1 for a follow which adds the response codes and documents the response schemas better.
|
I opened https://issues.apache.org/jira/browse/KAFKA-13976 for tracking the follow up items |
…-2022 * apache/trunk: (52 commits) KAFKA-13967: Document guarantees for producer callbacks on transaction commit (apache#12264) [KAFKA-13848] Clients remain connected after SASL re-authentication f… (apache#12179) KAFKA-10000: Zombie fencing logic (apache#11779) KAFKA-13947: Use %d formatting for integers rather than %s (apache#12267) KAFKA-13929: Replace legacy File.createNewFile() with NIO.2 Files.createFile() (apache#12197) KAFKA-13780: Generate OpenAPI file for Connect REST API (apache#12067) KAFKA-13917: Avoid calling lookupCoordinator() in tight loop (apache#12180) KAFKA-10199: Implement removing active and standby tasks from the state updater (apache#12270) MINOR: Update Scala to 2.13.8 in gradle.properties (apache#12273) MINOR: add java 8/scala 2.12 deprecation info in doc (apache#12261) ... Conflicts: gradle.properties
New gradle task: connect:runtime:genConnectOpenAPIDocs that generates connect_rest.yaml under docs/generated
This task is executed when siteDocsTar runs.
Committer Checklist (excluded from commit message)