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 2021/02/24 16:37:49 UTC
[GitHub] [kafka] ijuma opened a new pull request #10203: MINOR: Prepare for Gradle 7.0
ijuma opened a new pull request #10203:
URL: https://github.com/apache/kafka/pull/10203
Gradle 7.0 is required for Java 16 compatibility and it removes a number of
deprecated APIs. Update the build not to use deprecated APIs.
### 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: KAFKA-12415: Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r593221711
##########
File path: README.md
##########
@@ -69,10 +69,6 @@ Generate coverage for a single module, i.e.:
### Building a binary release gzipped tar ball ###
./gradlew clean releaseTarGz
-The above command will fail if you haven't set up the signing key. To bypass signing the artifact, you can run:
-
- ./gradlew clean releaseTarGz -x signArchives
Review comment:
Note that your command has `install` while the README line I deleted did not (and was wrong). It is still weird that you're seeing this for snapshot builds. I'll work with you all offline to try and figure it out.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch edited a comment on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch edited a comment on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-789538085
A few more things regarding `./gradlewAll publishToMavenLocal ` or `./gradlewAll install`:
1. These do not appear to publish source, test or test source JARs to the local Maven repository, unlike `./gradlewAll install` on the `2.8` branch.
2. Is it intentional to no longer publish `.asc` files to the local Maven repository?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583381642
##########
File path: README.md
##########
@@ -167,7 +167,7 @@ Please note for this to work you should create/update user maven settings (typic
### Installing the jars to the local Maven repository ###
- ./gradlewAll install
+ ./gradlewAll publishToMavenLocal
Review comment:
not sure whether this change is a kind of broken?
##########
File path: build.gradle
##########
@@ -1075,7 +1090,8 @@ project(':examples') {
archivesBaseName = "kafka-examples"
dependencies {
- compile project(':core')
+ implementation project(':clients')
Review comment:
Is this dependency redundant? The core module has `API` dependency from clients module so clients module is already propagated to metadata module.
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
I'm worry that connector projects which reference `connect:runtime` module to include other connect modules (i.e `connect:api` and `connect:json`) get broken by this change.
##########
File path: build.gradle
##########
@@ -219,34 +223,43 @@ subprojects {
options.compilerArgs << "--release" << minJavaVersion
}
- uploadArchives {
- repositories {
- signing {
- required { shouldSign }
- sign configurations.archives
-
- // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
- mavenDeployer {
- beforeDeployment { MavenDeployment deployment -> signing.signPom(deployment) }
- repository(url: "${mavenUrl}") {
- authentication(userName: "${mavenUsername}", password: "${mavenPassword}")
- }
- afterEvaluate {
- pom.artifactId = "${archivesBaseName}"
- pom.project {
- name 'Apache Kafka'
- packaging 'jar'
- url 'https://kafka.apache.org'
- licenses {
- license {
- name 'The Apache Software License, Version 2.0'
- url 'https://www.apache.org/licenses/LICENSE-2.0.txt'
- distribution 'repo'
- }
- }
- }
+ if (shouldPublish) {
+
+ publishing {
+ repositories {
+ // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
Review comment:
the `mavenUrl` is configurable (`-PmavenUrl=xxx`) so this comment is a bit weird.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586597356
##########
File path: build.gradle
##########
@@ -1833,27 +1848,30 @@ project(':jmh-benchmarks') {
apply plugin: 'com.github.johnrengelman.shadow'
shadowJar {
- baseName = 'kafka-jmh-benchmarks-all'
- classifier = null
Review comment:
Those changes rename the shadow jar from `kafka-jmh-benchmarks-all.jar` to `kafka-jmh-benchmarks-all-3.0.0-SNAPSHOT-all.jar`. That breaks `jmh.sh` as the script presumes the name of shadow jar is `kafka-jmh-benchmarks-all.jar` (https://github.com/apache/kafka/blob/trunk/jmh-benchmarks/jmh.sh#L40)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r584160375
##########
File path: build.gradle
##########
@@ -1401,33 +1416,35 @@ project(':streams') {
ext.buildStreamsVersionFileName = "kafka-streams-version.properties"
dependencies {
- compile project(':clients')
+ api project(':clients')
// this dependency should be removed after we unify data API
- compile(project(':connect:json')) {
+ implementation(project(':connect:json')) {
Review comment:
Updated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-789819700
The libs in distribution get changed.
### before
<img width="415" alt="截圖 2021-03-03 下午11 53 47" src="https://user-images.githubusercontent.com/6234750/109832849-ba19b280-7c7b-11eb-8a8a-9190f72a9ef7.png">
### after
<img width="369" alt="截圖 2021-03-03 下午11 53 55" src="https://user-images.githubusercontent.com/6234750/109832862-bf76fd00-7c7b-11eb-8280-a0c3db612b9f.png">
Not sure whether it breaks something (for example, users want to run main class from test code?)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583613236
##########
File path: build.gradle
##########
@@ -1075,7 +1090,8 @@ project(':examples') {
archivesBaseName = "kafka-examples"
dependencies {
- compile project(':core')
+ implementation project(':clients')
Review comment:
It's good practice to specify all direct dependencies and this module directly depends on `clients` for a lot of the examples.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch edited a comment on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch edited a comment on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-789538085
A few more things regarding `./gradlewAll publishToMavenLocal ` or `./gradlewAll install`:
1. Using `./gradlewAll publish` or `./gradlewAll install` does not appear to publish source, test or test source JARs to the local Maven repository, unlike `./gradlewAll install` on the `2.8` branch.
2. Is it intentional to no longer publish `.asc` files to the local Maven repository?
3. I did see the following warning strewn thru the output:
```
Cannot upload checksum for module-maven-metadata.xml because the remote repository doesn't support sha-512. This will not fail the build.
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma edited a comment on pull request #10203: KAFKA-12415 Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ijuma edited a comment on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-790636038
@chia7712 I filed https://issues.apache.org/jira/browse/KAFKA-12418 to follow up on the jars that are no longer included in the release tarball. It seems OK to me, but I'll do a bit more due dilligence before the 3.0.0 release.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-785751671
@chia7712 @omkreddy Any of you have cycles to review this? We reached the end of the road when it comes to delaying the handling of these Gradle deprecations. :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586713849
##########
File path: build.gradle
##########
@@ -1833,27 +1848,30 @@ project(':jmh-benchmarks') {
apply plugin: 'com.github.johnrengelman.shadow'
shadowJar {
- baseName = 'kafka-jmh-benchmarks-all'
- classifier = null
Review comment:
Good point, fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10203: KAFKA-12415: Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r592825616
##########
File path: README.md
##########
@@ -69,10 +69,6 @@ Generate coverage for a single module, i.e.:
### Building a binary release gzipped tar ball ###
./gradlew clean releaseTarGz
-The above command will fail if you haven't set up the signing key. To bypass signing the artifact, you can run:
-
- ./gradlew clean releaseTarGz -x signArchives
Review comment:
Yes, we're suddenly seeing our soak test (an application which deploys from trunk) fail with
```
./gradlew clean install releaseTarGz
> Configure project :
Building project 'core' with Scala version 2.13.5
Building project 'streams-scala' with Scala version 2.13.5
> Task :connect:signMavenJavaPublication FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':connect:signMavenJavaPublication'.
> Cannot perform signing task ':connect:signMavenJavaPublication' because it has no configured signatory
```
I personally haven't been able to reproduce this, but both @wcarlson5 and @lct45 have reported running into this issue
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586524651
##########
File path: build.gradle
##########
@@ -933,49 +953,49 @@ project(':core') {
':connect:runtime:genSinkConnectorConfigDocs', ':connect:runtime:genSourceConnectorConfigDocs',
':streams:genStreamsConfigDocs', 'genConsumerMetricsDocs', 'genProducerMetricsDocs',
':connect:runtime:genConnectMetricsDocs'], type: Tar) {
- classifier = 'site-docs'
+ archiveClassifier = 'site-docs'
compression = Compression.GZIP
from project.file("$rootDir/docs")
into 'site-docs'
duplicatesStrategy 'exclude'
}
tasks.create(name: "releaseTarGz", dependsOn: configurations.archives.artifacts, type: Tar) {
- into "kafka_${versions.baseScala}-${version}"
+ into "kafka_${versions.baseScala}-${archiveVersion}"
Review comment:
the `archiveVersion` shows weird string (see attachment). It is probably related to https://github.com/gradle/gradle/issues/14972
<img width="452" alt="截圖 2021-03-03 下午11 41 25" src="https://user-images.githubusercontent.com/6234750/109831016-019f3f00-7c7a-11eb-8e2c-5d785054bd11.png">
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583834500
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
The difference is that `connect-runtime` doesn't expose `connect-api` and others transitively anymore. If `connect-runtime` is used for tests where people _only_ depend on `connect-runtime` (this is an anti-pattern, but easy to use), things may break for them. That is the reason why I used `api` for the `clients` dependency in `core`, so I'll do the same for `connect-runtime`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r585894119
##########
File path: build.gradle
##########
@@ -1586,15 +1605,17 @@ project(':streams:test-utils') {
archivesBaseName = "kafka-streams-test-utils"
dependencies {
- compile project(':streams')
- compile project(':clients')
+ implementation project(':streams')
Review comment:
Yes, agreed. This is even more important since this module is public API and it exposes classes from streams and clients in said API (e.g. `Deserializer` and `TopologyException`).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r584164200
##########
File path: build.gradle
##########
@@ -1104,33 +1127,24 @@ project(':generator') {
project(':clients') {
archivesBaseName = "kafka-clients"
- configurations {
- jacksonDatabindConfig
- }
-
- // add jacksonDatabindConfig as provided scope config with high priority (1000)
- conf2ScopeMappings.addMapping(1000, configurations.jacksonDatabindConfig, "provided")
-
dependencies {
- compile libs.zstd
- compile libs.lz4
- compile libs.snappy
- compile libs.slf4jApi
+ implementation libs.zstd
Review comment:
you are right :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583612163
##########
File path: build.gradle
##########
@@ -219,34 +223,43 @@ subprojects {
options.compilerArgs << "--release" << minJavaVersion
}
- uploadArchives {
- repositories {
- signing {
- required { shouldSign }
- sign configurations.archives
-
- // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
- mavenDeployer {
- beforeDeployment { MavenDeployment deployment -> signing.signPom(deployment) }
- repository(url: "${mavenUrl}") {
- authentication(userName: "${mavenUsername}", password: "${mavenPassword}")
- }
- afterEvaluate {
- pom.artifactId = "${archivesBaseName}"
- pom.project {
- name 'Apache Kafka'
- packaging 'jar'
- url 'https://kafka.apache.org'
- licenses {
- license {
- name 'The Apache Software License, Version 2.0'
- url 'https://www.apache.org/licenses/LICENSE-2.0.txt'
- distribution 'repo'
- }
- }
- }
+ if (shouldPublish) {
+
+ publishing {
+ repositories {
+ // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
Review comment:
Yeah, you can pass -PmavenUrl or update gradle.properties. This was an existing comment, would you prefer if we suggest `-PmavenUrl` 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r585246882
##########
File path: build.gradle
##########
@@ -1586,15 +1605,17 @@ project(':streams:test-utils') {
archivesBaseName = "kafka-streams-test-utils"
dependencies {
- compile project(':streams')
- compile project(':clients')
+ implementation project(':streams')
Review comment:
I guess this is similar to `connect-runtime` that it is easy to be used in users' testing scope. Maybe we should keep exposing both modules.
##########
File path: release.py
##########
@@ -631,7 +631,7 @@ def select_gpg_key():
contents = f.read()
if not user_ok("Going to build and upload mvn artifacts based on these settings:\n" + contents + '\nOK (y/n)?: '):
fail("Retry again later")
-cmd("Building and uploading archives", "./gradlewAll uploadArchives", cwd=kafka_dir, env=jdk8_env, shell=True)
+cmd("Building and uploading archives", "./gradlewAll publish", cwd=kafka_dir, env=jdk8_env, shell=True)
Review comment:
the root/`README.md` still use `./gradlewAll uploadArchives`. It would be great to make them consistent.
##########
File path: build.gradle
##########
@@ -1468,7 +1461,7 @@ project(':streams') {
include('log4j*jar')
include('*hamcrest*')
}
- from (configurations.runtime) {
+ from (configurations.runtimeClasspath) {
Review comment:
Is there a jira already?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586553246
##########
File path: build.gradle
##########
@@ -933,49 +953,49 @@ project(':core') {
':connect:runtime:genSinkConnectorConfigDocs', ':connect:runtime:genSourceConnectorConfigDocs',
':streams:genStreamsConfigDocs', 'genConsumerMetricsDocs', 'genProducerMetricsDocs',
':connect:runtime:genConnectMetricsDocs'], type: Tar) {
- classifier = 'site-docs'
+ archiveClassifier = 'site-docs'
compression = Compression.GZIP
from project.file("$rootDir/docs")
into 'site-docs'
duplicatesStrategy 'exclude'
}
tasks.create(name: "releaseTarGz", dependsOn: configurations.archives.artifacts, type: Tar) {
- into "kafka_${versions.baseScala}-${version}"
+ into "kafka_${versions.baseScala}-${archiveVersion}"
Review comment:
Can you elaborate on how to reproduce this? When I run `releaseTarGz`, I see the site docs under a "distributions" folder.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r587471462
##########
File path: build.gradle
##########
@@ -1987,28 +2005,30 @@ project(':connect:json') {
archivesBaseName = "connect-json"
dependencies {
- compile project(':connect:api')
- compile libs.jacksonDatabind
- compile libs.jacksonJDK8Datatypes
- compile libs.slf4jApi
+ implementation project(':connect:api')
Review comment:
To clarify, `compile` is more like `api` than `implementation`. I switched the dependency of `connect-api` for `connect-json` and `connect-transform` to `api` for now. We can change this in a future PR, if we like, but it seems more consistent with the generally conservative approach we're taking in this PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583637868
##########
File path: README.md
##########
@@ -167,7 +167,7 @@ Please note for this to work you should create/update user maven settings (typic
### Installing the jars to the local Maven repository ###
- ./gradlewAll install
+ ./gradlewAll publishToMavenLocal
Review comment:
oh, my point was that ‘install’ is gone and it may break downstream QA which test code with latest source of kafka.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586853160
##########
File path: build.gradle
##########
@@ -1987,28 +2005,30 @@ project(':connect:json') {
archivesBaseName = "connect-json"
dependencies {
- compile project(':connect:api')
- compile libs.jacksonDatabind
- compile libs.jacksonJDK8Datatypes
- compile libs.slf4jApi
+ implementation project(':connect:api')
Review comment:
The public API is defined in `connect:api`, and `connect:json` and `connect:transforms` are implementations of interfaces defined in (and therefore cannot be used without) `connect:api`. Within AK, we provide the `connect:json` and `connect:transforms` always with the `connect:api` (and runtime) modules, and therefore using `implementation` (and `compile` in earlier branches) seems appropriate.
From a more abstract perspective, one might argue that the `connect:json` and `connect:transforms` could declare their use of `connect:api` as a transitive dependency (e.g., `api` rather than `implementation`). It's just that as a practical matter we don't need this, and arguably no other projects would either.
As such, I think it's safer to use `implementation`, as it's in line with what we were doing earlier.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r587468307
##########
File path: build.gradle
##########
@@ -1833,27 +1848,30 @@ project(':jmh-benchmarks') {
apply plugin: 'com.github.johnrengelman.shadow'
shadowJar {
- baseName = 'kafka-jmh-benchmarks-all'
- classifier = null
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-789798301
Thanks for testing @rhauch!
> @ijuma, I ran `./gradlew clean install releaseTarGz -x signArchives` (which is still in the README) and it failed with:
The readme was wrong when it comes to this since signing was only triggered when `uploadArchives` was called. I updated the readme instead of making code changes.
> These do not appear to publish source, test or test source JARs to the local Maven repository, unlike ./gradlewAll install on the 2.8 branch.
Good catch, I updated the build so that this works with the new plugin.
> Is it intentional to no longer publish .asc files to the local Maven repository? I do see with this PR the install commands now publish .module files that contain the various hashes, but I don't know if that's expected.
It seems like the new plugin only includes those when `publish` is called instead of `publishToMavenLocal`. That seems fine, I don't see any reason why those files are useful when installing to the local repo.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #10203: KAFKA-12415 Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-790636038
@chia7712 I filed https://issues.apache.org/jira/browse/KAFKA-12418 to follow up on the jars that are no longer included in the release tarball. It seems OK to me, but I'll do a bit more due dilligence before the 3.0 release.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: KAFKA-12415: Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r592821618
##########
File path: README.md
##########
@@ -69,10 +69,6 @@ Generate coverage for a single module, i.e.:
### Building a binary release gzipped tar ball ###
./gradlew clean releaseTarGz
-The above command will fail if you haven't set up the signing key. To bypass signing the artifact, you can run:
-
- ./gradlew clean releaseTarGz -x signArchives
Review comment:
Signing is skipped by default. Are you seeing something different? There is a table that describes `skipSigning`:
* https://github.com/apache/kafka#common-build-options
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: KAFKA-12415 Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r587485903
##########
File path: build.gradle
##########
@@ -1468,7 +1461,7 @@ project(':streams') {
include('log4j*jar')
include('*hamcrest*')
}
- from (configurations.runtime) {
+ from (configurations.runtimeClasspath) {
Review comment:
https://issues.apache.org/jira/browse/KAFKA-12417
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r584160401
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
@rhauch Can you take a look at the connect build changes and check if it's all 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586623841
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
The Connect portion looks good, so approving on this aspect. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583616719
##########
File path: README.md
##########
@@ -167,7 +167,7 @@ Please note for this to work you should create/update user maven settings (typic
### Installing the jars to the local Maven repository ###
- ./gradlewAll install
+ ./gradlewAll publishToMavenLocal
Review comment:
Can you elaborate please? It worked for me (it may require a `./gradlew clean` first 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586713849
##########
File path: build.gradle
##########
@@ -1833,27 +1848,30 @@ project(':jmh-benchmarks') {
apply plugin: 'com.github.johnrengelman.shadow'
shadowJar {
- baseName = 'kafka-jmh-benchmarks-all'
- classifier = null
Review comment:
Good point, fixed the script and readme.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-787104240
@mjsax @guozhangwang @vvcephei Can one of you review the build changes for the stream modules and check if they all make sense?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583640260
##########
File path: build.gradle
##########
@@ -219,34 +223,43 @@ subprojects {
options.compilerArgs << "--release" << minJavaVersion
}
- uploadArchives {
- repositories {
- signing {
- required { shouldSign }
- sign configurations.archives
-
- // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
- mavenDeployer {
- beforeDeployment { MavenDeployment deployment -> signing.signPom(deployment) }
- repository(url: "${mavenUrl}") {
- authentication(userName: "${mavenUsername}", password: "${mavenPassword}")
- }
- afterEvaluate {
- pom.artifactId = "${archivesBaseName}"
- pom.project {
- name 'Apache Kafka'
- packaging 'jar'
- url 'https://kafka.apache.org'
- licenses {
- license {
- name 'The Apache Software License, Version 2.0'
- url 'https://www.apache.org/licenses/LICENSE-2.0.txt'
- distribution 'repo'
- }
- }
- }
+ if (shouldPublish) {
+
+ publishing {
+ repositories {
+ // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
Review comment:
Yes, I prefer the way which need not to change source code :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r582115743
##########
File path: build.gradle
##########
@@ -219,37 +215,37 @@ subprojects {
options.compilerArgs << "--release" << minJavaVersion
}
- uploadArchives {
- repositories {
- signing {
- required { shouldSign }
- sign configurations.archives
-
- // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
- mavenDeployer {
- beforeDeployment { MavenDeployment deployment -> signing.signPom(deployment) }
- repository(url: "${mavenUrl}") {
- authentication(userName: "${mavenUsername}", password: "${mavenPassword}")
- }
- afterEvaluate {
- pom.artifactId = "${archivesBaseName}"
- pom.project {
- name 'Apache Kafka'
- packaging 'jar'
- url 'https://kafka.apache.org'
- licenses {
- license {
- name 'The Apache Software License, Version 2.0'
- url 'https://www.apache.org/licenses/LICENSE-2.0.txt'
- distribution 'repo'
- }
- }
- }
- }
- }
- }
- }
- }
+// uploadArchives {
+// repositories {
+// signing {
+// required { shouldSign }
+// sign configurations.archives
+//
+// // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
+// mavenDeployer {
+// beforeDeployment { MavenDeployment deployment -> signing.signPom(deployment) }
+// repository(url: "${mavenUrl}") {
+// authentication(userName: "${mavenUsername}", password: "${mavenPassword}")
+// }
+// afterEvaluate {
+// pom.artifactId = "${archivesBaseName}"
+// pom.project {
+// name 'Apache Kafka'
+// packaging 'jar'
+// url 'https://kafka.apache.org'
+// licenses {
+// license {
+// name 'The Apache Software License, Version 2.0'
+// url 'https://www.apache.org/licenses/LICENSE-2.0.txt'
+// distribution 'repo'
+// }
+// }
+// }
+// }
+// }
+// }
+// }
+// }
Review comment:
Need to update this to use equivalent maven-publish configuration.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583780438
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
Connector projects do require the `connect-api` module as a compile-time dependency and should not depend on `connect-runtime` as a compile-time dependency. Most (if not all) will include `connect-runtime` as a *test* dependency, though.
I don't really see anything that changed in the compile-time dependencies relative to the base commit. In particular, `connect-api` was included (on line 2030) before and is again. I think the removal of the blank line (2029) caused this diff page to render a bit strange.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583694082
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
ya, I agree that referencing connect-runtime in compile time is weird. However, core module is allowed to propagate client module. It would be better to get consistent rule for both connect-runtime and core.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r587471462
##########
File path: build.gradle
##########
@@ -1987,28 +2005,30 @@ project(':connect:json') {
archivesBaseName = "connect-json"
dependencies {
- compile project(':connect:api')
- compile libs.jacksonDatabind
- compile libs.jacksonJDK8Datatypes
- compile libs.slf4jApi
+ implementation project(':connect:api')
Review comment:
To clarify, `compile` is more like `api` than `implementation`. I switched the dependency of `connect-api` for `connect-json` and `connect-transform` to `api` for now. We can change this in a future API, if we like, but it seems more consistent with the generally conservative approach we're taking in this PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch edited a comment on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch edited a comment on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-789538085
A few more things regarding `./gradlewAll publishToMavenLocal ` or `./gradlewAll install`:
1. These do not appear to publish source, test or test source JARs to the local Maven repository, unlike `./gradlewAll install` on the `2.8` branch.
2. Is it intentional to no longer publish `.asc` files to the local Maven repository?
I have Gradle 6.8.3 installed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586553980
##########
File path: build.gradle
##########
@@ -933,49 +953,49 @@ project(':core') {
':connect:runtime:genSinkConnectorConfigDocs', ':connect:runtime:genSourceConnectorConfigDocs',
':streams:genStreamsConfigDocs', 'genConsumerMetricsDocs', 'genProducerMetricsDocs',
':connect:runtime:genConnectMetricsDocs'], type: Tar) {
- classifier = 'site-docs'
+ archiveClassifier = 'site-docs'
compression = Compression.GZIP
from project.file("$rootDir/docs")
into 'site-docs'
duplicatesStrategy 'exclude'
}
tasks.create(name: "releaseTarGz", dependsOn: configurations.archives.artifacts, type: Tar) {
- into "kafka_${versions.baseScala}-${version}"
+ into "kafka_${versions.baseScala}-${archiveVersion}"
Review comment:
Oh, I see, it's after extraction.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583780438
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
Connector projects do require the `connect-api` module as a compile-time dependency, but most (if not all) will include `connect-runtime` as a test dependency so that they can run their connector in Connect runtime.
I don't really see anything that changed in the compile-time dependencies relative to the base commit. In particular, `connect-api` was included (on line 2030) before and is again. I think the removal of the blank line (2029) caused this diff page to render a bit strange.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r584159269
##########
File path: build.gradle
##########
@@ -1104,33 +1127,24 @@ project(':generator') {
project(':clients') {
archivesBaseName = "kafka-clients"
- configurations {
- jacksonDatabindConfig
- }
-
- // add jacksonDatabindConfig as provided scope config with high priority (1000)
- conf2ScopeMappings.addMapping(1000, configurations.jacksonDatabindConfig, "provided")
-
dependencies {
- compile libs.zstd
- compile libs.lz4
- compile libs.snappy
- compile libs.slf4jApi
+ implementation libs.zstd
Review comment:
Applications should not have a compile time dependency to these libraries, right? They would have a runtime dependency.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #10203: KAFKA-12415 Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-790865459
No test failures, but one of the jobs was killed in the last run:
https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-10203/19/testReport/
The previous runs confirm that it's a flaky issue unrelated to this PR (there are no test changes here).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10203: KAFKA-12415: Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r592825616
##########
File path: README.md
##########
@@ -69,10 +69,6 @@ Generate coverage for a single module, i.e.:
### Building a binary release gzipped tar ball ###
./gradlew clean releaseTarGz
-The above command will fail if you haven't set up the signing key. To bypass signing the artifact, you can run:
-
- ./gradlew clean releaseTarGz -x signArchives
Review comment:
Yes, we're suddenly seeing our soak test applications which deploys from trunk fail with
```
./gradlew clean install releaseTarGz
> Configure project :
Building project 'core' with Scala version 2.13.5
Building project 'streams-scala' with Scala version 2.13.5
> Task :connect:signMavenJavaPublication FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':connect:signMavenJavaPublication'.
> Cannot perform signing task ':connect:signMavenJavaPublication' because it has no configured signatory
```
I personally haven't been able to reproduce this, but both @wcarlson5 and @lct45 have reported running into this issue
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586554506
##########
File path: build.gradle
##########
@@ -933,49 +953,49 @@ project(':core') {
':connect:runtime:genSinkConnectorConfigDocs', ':connect:runtime:genSourceConnectorConfigDocs',
':streams:genStreamsConfigDocs', 'genConsumerMetricsDocs', 'genProducerMetricsDocs',
':connect:runtime:genConnectMetricsDocs'], type: Tar) {
- classifier = 'site-docs'
+ archiveClassifier = 'site-docs'
compression = Compression.GZIP
from project.file("$rootDir/docs")
into 'site-docs'
duplicatesStrategy 'exclude'
}
tasks.create(name: "releaseTarGz", dependsOn: configurations.archives.artifacts, type: Tar) {
- into "kafka_${versions.baseScala}-${version}"
+ into "kafka_${versions.baseScala}-${archiveVersion}"
Review comment:
> I see the site docs under a "distributions" folder.
untar the `kafka_2.13-3.0.0-SNAPSHOT.tgz` :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma merged pull request #10203: KAFKA-12415 Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ijuma merged pull request #10203:
URL: https://github.com/apache/kafka/pull/10203
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ableegoldman commented on a change in pull request #10203: KAFKA-12415: Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r592808364
##########
File path: README.md
##########
@@ -69,10 +69,6 @@ Generate coverage for a single module, i.e.:
### Building a binary release gzipped tar ball ###
./gradlew clean releaseTarGz
-The above command will fail if you haven't set up the signing key. To bypass signing the artifact, you can run:
-
- ./gradlew clean releaseTarGz -x signArchives
Review comment:
@ijuma the proper way to skip signing is now to run `./gradlew clean releaseTarGz -DskipSigning`, is that right? Is there a reason we removed this instruction from the README instead of updating 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r584141982
##########
File path: build.gradle
##########
@@ -1401,33 +1416,35 @@ project(':streams') {
ext.buildStreamsVersionFileName = "kafka-streams-version.properties"
dependencies {
- compile project(':clients')
+ api project(':clients')
// this dependency should be removed after we unify data API
- compile(project(':connect:json')) {
+ implementation(project(':connect:json')) {
Review comment:
ditto
##########
File path: build.gradle
##########
@@ -1104,33 +1127,24 @@ project(':generator') {
project(':clients') {
archivesBaseName = "kafka-clients"
- configurations {
- jacksonDatabindConfig
- }
-
- // add jacksonDatabindConfig as provided scope config with high priority (1000)
- conf2ScopeMappings.addMapping(1000, configurations.jacksonDatabindConfig, "provided")
-
dependencies {
- compile libs.zstd
- compile libs.lz4
- compile libs.snappy
- compile libs.slf4jApi
+ implementation libs.zstd
Review comment:
How about exposing those compression libraries for compatibility?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch edited a comment on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch edited a comment on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-789538085
A few more things regarding `./gradlewAll publishToMavenLocal ` or `./gradlewAll install`:
1. These do not appear to publish source, test or test source JARs to the local Maven repository, unlike `./gradlewAll install` on the `2.8` branch.
2. Is it intentional to no longer publish `.asc` files to the local Maven repository? I do see with this PR the install commands now publish `.module` files that contain the various hashes, but I don't know if that's expected.
I have Gradle 6.8.3 installed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: KAFKA-12415: Prepare for Gradle 7.0 and restrict transitive scope for non api dependencies
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r593271376
##########
File path: README.md
##########
@@ -69,10 +69,6 @@ Generate coverage for a single module, i.e.:
### Building a binary release gzipped tar ball ###
./gradlew clean releaseTarGz
-The above command will fail if you haven't set up the signing key. To bypass signing the artifact, you can run:
-
- ./gradlew clean releaseTarGz -x signArchives
Review comment:
We figured it out, the build was using a non snapshot version. Passing `-PskipSigning=true` fixed it. Submitted https://github.com/apache/kafka/pull/10307 to make it easier to debug these issues in the future.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583780438
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
Connector projects do require the `connect-api` module as a compile-time dependency, but most (if not all) will include `connect-runtime` as a test dependency so that they can run their connector in Connect runtime.
I don't really see anything that changed relative to the compile-time dependencies. In particular, `connect-api` was included before and is again, but the blank line (2029) was removed, causing this diff page to render a bit strange.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch edited a comment on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch edited a comment on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-789538085
A few more things regarding `./gradlewAll publishToMavenLocal ` or `./gradlewAll install`:
1. These do not appear to publish source, test or test source JARs to the local Maven repository, unlike `./gradlewAll install` on the `2.8` branch.
2. Is it intentional to no longer publish `.asc` files to the local Maven repository?
3. I did see the following warning strewn thru the output:
```
Cannot upload checksum for module-maven-metadata.xml because the remote repository doesn't support sha-512. This will not fail the build.
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r582117515
##########
File path: build.gradle
##########
@@ -1468,7 +1461,7 @@ project(':streams') {
include('log4j*jar')
include('*hamcrest*')
}
- from (configurations.runtime) {
+ from (configurations.runtimeClasspath) {
Review comment:
We should also change `configurations.testRuntime` a few lines above to `configurations.testRuntimeClasspath`, but this causes a cyclic build error. Fixing this will be required before we move to Gradle 7.0.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch commented on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch commented on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-789538085
A few more things regarding `./gradlewAll publish` or `./gradlewAll install`:
1. Using `./gradlewAll publish` or `./gradlewAll install` does not appear to publish source, test or test source JARs to the local Maven repository, unlike `./gradlewAll install` on the `2.8` branch.
2. Is it intentional to no longer publish `.asc` files to the local Maven repository?
3. I did see the following warning strewn thru the output:
```
Cannot upload checksum for module-maven-metadata.xml because the remote repository doesn't support sha-512. This will not fail the build.
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583678979
##########
File path: README.md
##########
@@ -167,7 +167,7 @@ Please note for this to work you should create/update user maven settings (typic
### Installing the jars to the local Maven repository ###
- ./gradlewAll install
+ ./gradlewAll publishToMavenLocal
Review comment:
Makes sense. I added task aliases and also converted one case to the new commands.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583981010
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
Ah, I see that now. If we wanted to be strict, we could leave as an implementation dependency. However, I agree that it's safest to use `api` for the `connect-api` module.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-788083914
@chia7712 Are you ok after the latest 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r585891145
##########
File path: release.py
##########
@@ -631,7 +631,7 @@ def select_gpg_key():
contents = f.read()
if not user_ok("Going to build and upload mvn artifacts based on these settings:\n" + contents + '\nOK (y/n)?: '):
fail("Retry again later")
-cmd("Building and uploading archives", "./gradlewAll uploadArchives", cwd=kafka_dir, env=jdk8_env, shell=True)
+cmd("Building and uploading archives", "./gradlewAll publish", cwd=kafka_dir, env=jdk8_env, shell=True)
Review comment:
Good catch. I had searched for this, but somehow missed 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r584139127
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
Fixed.
##########
File path: build.gradle
##########
@@ -219,34 +223,43 @@ subprojects {
options.compilerArgs << "--release" << minJavaVersion
}
- uploadArchives {
- repositories {
- signing {
- required { shouldSign }
- sign configurations.archives
-
- // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
- mavenDeployer {
- beforeDeployment { MavenDeployment deployment -> signing.signPom(deployment) }
- repository(url: "${mavenUrl}") {
- authentication(userName: "${mavenUsername}", password: "${mavenPassword}")
- }
- afterEvaluate {
- pom.artifactId = "${archivesBaseName}"
- pom.project {
- name 'Apache Kafka'
- packaging 'jar'
- url 'https://kafka.apache.org'
- licenses {
- license {
- name 'The Apache Software License, Version 2.0'
- url 'https://www.apache.org/licenses/LICENSE-2.0.txt'
- distribution 'repo'
- }
- }
- }
+ if (shouldPublish) {
+
+ publishing {
+ repositories {
+ // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
Review comment:
Fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r582672553
##########
File path: build.gradle
##########
@@ -21,45 +21,41 @@ buildscript {
repositories {
mavenCentral()
jcenter()
- maven {
- url "https://plugins.gradle.org/m2/"
- }
}
apply from: file('gradle/buildscript.gradle'), to: buildscript
apply from: "$rootDir/gradle/dependencies.gradle"
dependencies {
// For Apache Rat plugin to ignore non-Git files
classpath "org.ajoberstar.grgit:grgit-core:$versions.grgit"
- classpath "com.github.ben-manes:gradle-versions-plugin:$versions.gradleVersionsPlugin"
- classpath "org.scoverage:gradle-scoverage:$versions.scoveragePlugin"
- classpath "com.github.jengelman.gradle.plugins:shadow:$versions.shadowPlugin"
- classpath "org.owasp:dependency-check-gradle:$versions.owaspDepCheckPlugin"
- classpath "com.diffplug.spotless:spotless-plugin-gradle:$versions.spotlessPlugin"
- classpath "gradle.plugin.com.github.spotbugs.snom:spotbugs-gradle-plugin:$versions.spotbugsPlugin"
- classpath "org.gradle:test-retry-gradle-plugin:$versions.testRetryPlugin"
}
}
-apply plugin: "com.diffplug.spotless"
+plugins {
+ id 'com.diffplug.spotless' version '5.10.2'
Review comment:
We have to either include the version literal here or in a properties file. Methods cannot be invoked. Having a separate properties file and the `dependencies` file didn't seem particularly better than this way.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586642475
##########
File path: build.gradle
##########
@@ -1987,28 +2005,30 @@ project(':connect:json') {
archivesBaseName = "connect-json"
dependencies {
- compile project(':connect:api')
- compile libs.jacksonDatabind
- compile libs.jacksonJDK8Datatypes
- compile libs.slf4jApi
+ implementation project(':connect:api')
Review comment:
Does `connect-json` expose any `connect-api` types in its public API?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586569305
##########
File path: build.gradle
##########
@@ -933,49 +953,49 @@ project(':core') {
':connect:runtime:genSinkConnectorConfigDocs', ':connect:runtime:genSourceConnectorConfigDocs',
':streams:genStreamsConfigDocs', 'genConsumerMetricsDocs', 'genProducerMetricsDocs',
':connect:runtime:genConnectMetricsDocs'], type: Tar) {
- classifier = 'site-docs'
+ archiveClassifier = 'site-docs'
compression = Compression.GZIP
from project.file("$rootDir/docs")
into 'site-docs'
duplicatesStrategy 'exclude'
}
tasks.create(name: "releaseTarGz", dependsOn: configurations.archives.artifacts, type: Tar) {
- into "kafka_${versions.baseScala}-${version}"
+ into "kafka_${versions.baseScala}-${archiveVersion}"
Review comment:
Fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r587104104
##########
File path: build.gradle
##########
@@ -1833,27 +1848,30 @@ project(':jmh-benchmarks') {
apply plugin: 'com.github.johnrengelman.shadow'
shadowJar {
- baseName = 'kafka-jmh-benchmarks-all'
- classifier = null
Review comment:
Could we remove duplicate "all" from name of shadow jar? `kafka-jmh-benchmarks-all-3.0.0-SNAPSHOT-all.jar`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r586619026
##########
File path: build.gradle
##########
@@ -1987,28 +2005,30 @@ project(':connect:json') {
archivesBaseName = "connect-json"
dependencies {
- compile project(':connect:api')
- compile libs.jacksonDatabind
- compile libs.jacksonJDK8Datatypes
- compile libs.slf4jApi
+ implementation project(':connect:api')
Review comment:
I think this is the correct mapping.
By switching to `implementation`, any external project depending on the `connect:json` module will have to include an explicit dependency on `connect:api`. This appears to be have the same effect as what previous branches do with `compile` on earlier branches, but using `implementation` seems more correct since the [Gradle docs](https://docs.gradle.org/current/userguide/java_library_plugin.html) suggest that `compile` (or `compileOnly`) are more applicable for shading, which is not our case here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] rhauch commented on pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
rhauch commented on pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#issuecomment-789526689
@ijuma, I ran `./gradlew clean install releaseTarGz -x signArchives` (which is still in the README) and it failed with:
```
* What went wrong:
Task 'signArchives' not found in root project 'kafka'.
```
If that should not work, we should also update the README. But if it should work, we may need a bit more work for building an unsigned release archive.
FWIW, running `./gradew clean install releaseTarGz` and `./gradlewAll publish` both appear to work as expected.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r582115743
##########
File path: build.gradle
##########
@@ -219,37 +215,37 @@ subprojects {
options.compilerArgs << "--release" << minJavaVersion
}
- uploadArchives {
- repositories {
- signing {
- required { shouldSign }
- sign configurations.archives
-
- // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
- mavenDeployer {
- beforeDeployment { MavenDeployment deployment -> signing.signPom(deployment) }
- repository(url: "${mavenUrl}") {
- authentication(userName: "${mavenUsername}", password: "${mavenPassword}")
- }
- afterEvaluate {
- pom.artifactId = "${archivesBaseName}"
- pom.project {
- name 'Apache Kafka'
- packaging 'jar'
- url 'https://kafka.apache.org'
- licenses {
- license {
- name 'The Apache Software License, Version 2.0'
- url 'https://www.apache.org/licenses/LICENSE-2.0.txt'
- distribution 'repo'
- }
- }
- }
- }
- }
- }
- }
- }
+// uploadArchives {
+// repositories {
+// signing {
+// required { shouldSign }
+// sign configurations.archives
+//
+// // To test locally, replace mavenUrl in ~/.gradle/gradle.properties to file://localhost/tmp/myRepo/
+// mavenDeployer {
+// beforeDeployment { MavenDeployment deployment -> signing.signPom(deployment) }
+// repository(url: "${mavenUrl}") {
+// authentication(userName: "${mavenUsername}", password: "${mavenPassword}")
+// }
+// afterEvaluate {
+// pom.artifactId = "${archivesBaseName}"
+// pom.project {
+// name 'Apache Kafka'
+// packaging 'jar'
+// url 'https://kafka.apache.org'
+// licenses {
+// license {
+// name 'The Apache Software License, Version 2.0'
+// url 'https://www.apache.org/licenses/LICENSE-2.0.txt'
+// distribution 'repo'
+// }
+// }
+// }
+// }
+// }
+// }
+// }
+// }
Review comment:
Need to update this to use equivalent maven-publish configuration.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #10203: MINOR: Prepare for Gradle 7.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10203:
URL: https://github.com/apache/kafka/pull/10203#discussion_r583614195
##########
File path: build.gradle
##########
@@ -2026,52 +2043,53 @@ project(':connect:runtime') {
archivesBaseName = "connect-runtime"
dependencies {
-
- compile project(':connect:api')
- compile project(':clients')
- compile project(':tools')
- compile project(':connect:json')
- compile project(':connect:transforms')
-
- compile libs.slf4jApi
- compile libs.jacksonJaxrsJsonProvider
- compile libs.jerseyContainerServlet
- compile libs.jerseyHk2
- compile libs.jaxbApi // Jersey dependency that was available in the JDK before Java 9
- compile libs.activation // Jersey dependency that was available in the JDK before Java 9
- compile libs.jettyServer
- compile libs.jettyServlet
- compile libs.jettyServlets
- compile libs.jettyClient
- compile(libs.reflections)
- compile(libs.mavenArtifact)
-
- testCompile project(':clients').sourceSets.test.output
- testCompile libs.easymock
- testCompile libs.junitJupiterApi
- testCompile libs.junitVintageEngine
- testCompile libs.powermockJunit4
- testCompile libs.powermockEasymock
- testCompile libs.mockitoCore
- testCompile libs.httpclient
-
- testCompile project(':clients').sourceSets.test.output
- testCompile project(':core')
- testCompile project(':core').sourceSets.test.output
-
- testRuntime libs.slf4jlog4j
+ implementation project(':connect:api')
Review comment:
Thoughts @kkonstantine and @rhauch? I thought connectors were meant to reference `connect-api` for compilation and the runtime is what gets included at... runtime. :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org