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/09/09 14:59:09 UTC
[GitHub] [kafka] C0urante opened a new pull request, #12616: KAFKA-14198: Define separate swagger configuration
C0urante opened a new pull request, #12616:
URL: https://github.com/apache/kafka/pull/12616
Follow-up from https://github.com/apache/kafka/pull/12609. Non-urgent, just tidies up the build file a bit.
### 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] C0urante commented on pull request #12616: KAFKA-14198: Define separate swagger configuration in Gradle build
Posted by GitBox <gi...@apache.org>.
C0urante commented on PR #12616:
URL: https://github.com/apache/kafka/pull/12616#issuecomment-1243683309
Thanks @ijuma. Yes, I verified both, and updated the PR description with the steps I took to do so.
--
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] ijuma commented on pull request #12616: KAFKA-14198: Define separate swagger configuration
Posted by GitBox <gi...@apache.org>.
ijuma commented on PR #12616:
URL: https://github.com/apache/kafka/pull/12616#issuecomment-1242276303
Thanks @C0urante. Did you verify that the documentation is still generated correctly and the release tarball still doesn't have swagger and snakyaml in the dependencies?
--
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] ijuma commented on a diff in pull request #12616: KAFKA-14198: Define separate swagger configuration
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #12616:
URL: https://github.com/apache/kafka/pull/12616#discussion_r967324775
##########
build.gradle:
##########
@@ -2564,7 +2568,9 @@ project(':connect:runtime') {
implementation libs.mavenArtifact
implementation libs.swaggerAnnotations
Review Comment:
Do we need this to be `implementation` or can it be `swagger` 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
[GitHub] [kafka] C0urante commented on pull request #12616: KAFKA-14198: Define separate swagger configuration in Gradle build
Posted by GitBox <gi...@apache.org>.
C0urante commented on PR #12616:
URL: https://github.com/apache/kafka/pull/12616#issuecomment-1243995392
@ijuma @jsancio I'm assuming we don't need to backport this to 3.3 since the immediate issues with Swagger deps and Connect OpenAPI docs generation have already been fixed on that branch; let me know if that's not the case, though.
--
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 #12616: KAFKA-14198: Define separate swagger configuration in Gradle build
Posted by GitBox <gi...@apache.org>.
C0urante commented on PR #12616:
URL: https://github.com/apache/kafka/pull/12616#issuecomment-1243994114
Failures appear unrelated, merging.
--
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 #12616: KAFKA-14198: Define separate swagger configuration in Gradle build
Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12616:
URL: https://github.com/apache/kafka/pull/12616#discussion_r968366903
##########
build.gradle:
##########
@@ -2653,9 +2659,9 @@ project(':connect:runtime') {
}
task genConnectOpenAPIDocs(type: io.swagger.v3.plugins.gradle.tasks.ResolveTask, dependsOn: setVersionInOpenAPISpec) {
- classpath = sourceSets.main.compileClasspath + sourceSets.main.runtimeClasspath
+ classpath = sourceSets.main.runtimeClasspath
Review Comment:
The plugin requires the `classpath` field to be set.
I also tried this:
```
classpath = sourceSets.main.runtimeClasspath + configurations.swagger.asCollection()
buildClasspath = classpath
```
which led to a lot of these warnings:
> Gradle detected a problem with the following location... Reason: Task ':connect:runtime:genConnectOpenAPIDocs' uses this output of task ':connect:runtime:processResources' without declaring an explicit or implicit dependency.
Based on [discussion in the PR](https://github.com/apache/kafka/pull/12067#discussion_r892542837) that introduced OpenAPI docs generation for Connect, it appears that setting `classpath` to `sourceSets.main.runtimeClasspath` creates some implicit dependencies for the `genConnectOpenAPIDocs` task that fix these warnings.
--
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 #12616: KAFKA-14198: Define separate swagger configuration in Gradle build
Posted by GitBox <gi...@apache.org>.
C0urante merged PR #12616:
URL: https://github.com/apache/kafka/pull/12616
--
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] ijuma commented on a diff in pull request #12616: KAFKA-14198: Define separate swagger configuration
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #12616:
URL: https://github.com/apache/kafka/pull/12616#discussion_r967325334
##########
build.gradle:
##########
@@ -2653,9 +2659,9 @@ project(':connect:runtime') {
}
task genConnectOpenAPIDocs(type: io.swagger.v3.plugins.gradle.tasks.ResolveTask, dependsOn: setVersionInOpenAPISpec) {
- classpath = sourceSets.main.compileClasspath + sourceSets.main.runtimeClasspath
+ classpath = sourceSets.main.runtimeClasspath
Review Comment:
Do we need this assignment at all or can we just inline it?
--
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 #12616: KAFKA-14198: Define separate swagger configuration in Gradle build
Posted by GitBox <gi...@apache.org>.
C0urante commented on PR #12616:
URL: https://github.com/apache/kafka/pull/12616#issuecomment-1243801973
Thanks Ismael. Kicked off another Jenkins build since the previous one failed with a disk space error; will merge if everything looks alright.
--
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 #12616: KAFKA-14198: Define separate swagger configuration in Gradle build
Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12616:
URL: https://github.com/apache/kafka/pull/12616#discussion_r968359263
##########
build.gradle:
##########
@@ -2564,7 +2568,9 @@ project(':connect:runtime') {
implementation libs.mavenArtifact
implementation libs.swaggerAnnotations
Review Comment:
I gave this a try but it wasn't successful. We still need the annotations at compile time since they're used to decorate our REST resource classes, like here: https://github.com/apache/kafka/blob/b449b8032edb0e4e6b3213e38a38206a3048c6f5/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResource.java#L108-L114
--
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