Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BEAM-10961] Enabled strict dependency on sdks-extensions #13686

Closed
wants to merge 12 commits into from
Closed

[BEAM-10961] Enabled strict dependency on sdks-extensions #13686

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6c9550c
Enabled strict dependency on sdks-extensions
sonam-vend Jan 7, 2021
29b2183
move sdks/java/extensions/sql changes to their own PR
shehzaadn-vd Jan 28, 2021
57b9451
enableStrictDependencies and other cleanup
shehzaadn-vd Jan 28, 2021
3a2fa11
declare auto-value-annotations
shehzaadn-vd Jan 28, 2021
d794f6a
Merge branch 'master' into sdks-extensions
shehzaadn-vd Feb 6, 2021
b679b53
[BEAM-10961] (1) reintroduce previously removed compile lines paired …
shehzaadn-vd Feb 6, 2021
77fddd3
[BEAM-10961] some cleanup
shehzaadn-vd Feb 11, 2021
ac654b3
[BEAM-10961] remove library.java.slf4j_api from sdks/java/extensions/…
shehzaadn-vd Feb 13, 2021
73dfeae
merge with master
shehzaadn-vd Feb 20, 2021
7ff6f4f
trigger build
shehzaadn-vd Feb 24, 2021
740e349
[BEAM-10961] add reference to BEAM-11761
shehzaadn-vd Feb 25, 2021
619d078
Merge branch 'master' into sdks-extensions
shehzaadn-vd Feb 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion sdks/java/extensions/euphoria/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

plugins { id 'org.apache.beam.module' }
applyJavaNature(
enableStrictDependencies: true,
exportJavadoc: false,
automaticModuleName: 'org.apache.beam.sdk.extensions.euphoria',
)
Expand All @@ -26,7 +27,10 @@ description = "Apache Beam :: SDKs :: Java :: Extensions :: Euphoria Java 8 DSL"

dependencies {
compile project(path: ":sdks:java:core", configuration: "shadow")
testCompile library.java.mockito_core
compile library.java.jackson_annotations
compile library.java.joda_time
compile library.java.slf4j_api
compile library.java.vendored_guava_26_0_jre
testCompile project(":sdks:java:extensions:kryo")
testCompile library.java.slf4j_api
testCompile library.java.hamcrest_core
Expand Down
14 changes: 8 additions & 6 deletions sdks/java/extensions/google-cloud-platform-core/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import groovy.json.JsonOutput

plugins { id 'org.apache.beam.module' }
applyJavaNature(

enableStrictDependencies: true,
automaticModuleName: 'org.apache.beam.sdk.extensions.gcp')

description = "Apache Beam :: SDKs :: Java :: Extensions :: Google Cloud Platform Core"
Expand All @@ -40,23 +40,25 @@ dependencies {
compile library.java.vendored_guava_26_0_jre
compile project(path: ":sdks:java:core", configuration: "shadow")
compile project(path: ":runners:core-java")
compile library.java.bigdataoss_gcsio
compile library.java.bigdataoss_util
compile library.java.google_http_client_jackson2
compile library.java.google_auth_library_oauth2_http
compile library.java.google_api_client
compile library.java.bigdataoss_gcsio
compile library.java.bigdataoss_util
compile library.java.google_api_services_cloudresourcemanager
compile library.java.google_api_services_storage
compile library.java.google_auth_library_credentials
compile library.java.google_http_client
compile library.java.slf4j_api
compile library.java.joda_time
compile library.java.http_core
compile library.java.http_client
compile library.java.jackson_annotations
compile library.java.jackson_databind
permitUnusedDeclared library.java.jackson_databind // BEAM-11761
compile library.java.joda_time
compile library.java.slf4j_api
provided library.java.hamcrest_core
provided library.java.junit
testCompile project(path: ":sdks:java:core", configuration: "shadowTest")
testCompile library.java.hamcrest_library
testCompile library.java.mockito_core
testRuntimeOnly library.java.slf4j_jdk14
}
Expand Down
6 changes: 3 additions & 3 deletions sdks/java/extensions/jackson/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

plugins { id 'org.apache.beam.module' }
applyJavaNature(
enableStrictDependencies: true,
automaticModuleName: 'org.apache.beam.sdk.extensions.jackson',
archivesBaseName: 'beam-sdks-java-extensions-json-jackson'
)
Expand All @@ -26,11 +27,10 @@ description = "Apache Beam :: SDKs :: Java :: Extensions :: Jackson"
ext.summary = "Jackson extension provides PTransforms for deserializing and generating JSON strings."

dependencies {
compile library.java.vendored_guava_26_0_jre
compile project(path: ":sdks:java:core", configuration: "shadow")
compile library.java.jackson_databind
testCompile library.java.hamcrest_core
testCompile library.java.hamcrest_library
compile library.java.jackson_core
compile library.java.vendored_guava_26_0_jre
testCompile library.java.junit
testRuntimeOnly project(path: ":runners:direct-java", configuration: "shadow")
}
7 changes: 4 additions & 3 deletions sdks/java/extensions/join-library/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,16 @@
*/

plugins { id 'org.apache.beam.module' }
applyJavaNature(automaticModuleName: 'org.apache.beam.sdk.extensions.joinlibrary')
applyJavaNature(
enableStrictDependencies: true,
automaticModuleName: 'org.apache.beam.sdk.extensions.joinlibrary'
)

description = "Apache Beam :: SDKs :: Java :: Extensions :: Join library"

dependencies {
compile library.java.vendored_guava_26_0_jre
compile project(path: ":sdks:java:core", configuration: "shadow")
testCompile library.java.hamcrest_core
testCompile library.java.hamcrest_library
testCompile library.java.junit
testRuntimeOnly project(path: ":runners:direct-java", configuration: "shadow")
}
4 changes: 4 additions & 0 deletions sdks/java/extensions/kryo/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ ext {
}

applyJavaNature(automaticModuleName: 'org.apache.beam.sdk.extensions.kryo',
enableStrictDependencies: true,
exportJavadoc: false,
shadowClosure: {
dependencies {
Expand All @@ -39,7 +40,10 @@ applyJavaNature(automaticModuleName: 'org.apache.beam.sdk.extensions.kryo',
description = 'Apache Beam :: SDKs :: Java :: Extensions :: Kryo'

dependencies {
compile library.java.jackson_annotations
compile library.java.vendored_guava_26_0_jre
compile "com.esotericsoftware:kryo:${kryoVersion}"
compile "org.objenesis:objenesis:2.5.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting that these dependencies are also included in the Java nature configuratioin's shadowClosure.dependencies. I'm don't think it's a problem, but I'm curious what is the difference between specifying dependencies in shadowClosure vs the regular dependencies block.

shadow project(path: ':sdks:java:core', configuration: 'shadow')
testCompile project(path: ':sdks:java:core', configuration: 'shadowTest')
testRuntimeOnly project(path: ':runners:direct-java', configuration: 'shadow')
Expand Down
18 changes: 13 additions & 5 deletions sdks/java/extensions/ml/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,33 @@ import groovy.json.JsonOutput
*/

plugins { id 'org.apache.beam.module' }
applyJavaNature(automaticModuleName: 'org.apache.beam.sdk.extensions.ml')
applyJavaNature(
enableStrictDependencies: true,
automaticModuleName: 'org.apache.beam.sdk.extensions.ml'
)

description = 'Apache Beam :: SDKs :: Java :: Extensions :: ML'

dependencies {
compile project(path: ":sdks:java:core", configuration: "shadow")
compile project(":sdks:java:expansion-service")
permitUnusedDeclared project(":sdks:java:expansion-service")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please link the jira in a comment (BEAM-11761) for the permitUnusedDeclared instances we're not sure if we can remove.

compile 'com.google.cloud:google-cloud-video-intelligence:1.2.0'
compile 'com.google.cloud:google-cloud-dlp:1.1.4'
compile 'com.google.cloud:google-cloud-language:1.99.4'
compile 'com.google.api.grpc:proto-google-cloud-dlp-v2:1.1.4'
compile 'com.google.api.grpc:proto-google-cloud-language-v1:1.81.4'
compile 'com.google.api.grpc:proto-google-cloud-video-intelligence-v1:1.2.0'
compile 'com.google.api.grpc:proto-google-cloud-vision-v1:1.81.3'
compile library.java.gax
compile library.java.protobuf_java
compile library.java.slf4j_api
provided library.java.junit
testCompile project(path: ':sdks:java:core', configuration: 'shadowTest')
compile 'com.google.cloud:google-cloud-vision:1.99.3'
permitUsedUndeclared "com.google.auto.value:auto-value-annotations:1.7"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be permitUsedUndeclared? Don't we already declare it in BeamModulePlugin?

beam/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy

Line 870 in 705649d

"com.google.auto.value:auto-value-annotations:$autovalue_version",

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibzib When i remove this line, i get the error:

Execution failed for task ':sdks:java:extensions:ml:analyzeClassesDependencies'.
> Dependency analysis found issues.
  usedUndeclaredArtifacts: 
   - com.google.auto.value:auto-value-annotations:1.7@jar

To dig into this further, I ran ../../../../gradlew -q dependencies in sdks/java/extensions/ml and captured the output here:
https://docs.google.com/document/d/1FNxea7Qbn8ckrphV25U7qh75a6z-PEl3iKgdJJwld5E/edit

It seems that via com.google.cloud:google-cloud-video-intelligence:1.2.0, we are getting runtimeClasspath dependencies to com.google.auto.value:auto-value-annotations:1.7 - which is not being resolved to 1.7.2, the version declared in BeamModulePlugin.groovy as a compileOnly dependency. See the highlighted lines in the doc I linked to above.

Maybe this is because BeamModulePlugin.groovy declares auto-value-annotations as compileOnly whereas it is actually needed at runtime.

Not sure if this could point to the the cause or it's just a red herring...?

testCompile library.java.mockito_core
testCompile 'com.google.cloud:google-cloud-video-intelligence:1.2.0'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing these testCompile lines since the same dependencies already have compile lines.

testCompile 'com.google.cloud:google-cloud-dlp:1.1.4'
testCompile project(path: ":sdks:java:extensions:google-cloud-platform-core", configuration: "testRuntime")
testCompile 'com.google.cloud:google-cloud-language:1.99.4'
testCompile 'com.google.cloud:google-cloud-vision:1.99.3'
testRuntimeOnly project(path: ":runners:direct-java", configuration: "shadow")
testRuntimeOnly project(":runners:google-cloud-dataflow-java")
}
Expand Down
7 changes: 3 additions & 4 deletions sdks/java/extensions/protobuf/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

plugins { id 'org.apache.beam.module' }
applyJavaNature(
enableStrictDependencies: true,
generatedClassPatterns: [
/^org\.apache\.beam\.sdk\.extensions\.protobuf\.Proto2CoderTestMessages/,
/^org\.apache\.beam\.sdk\.extensions\.protobuf\.Proto2SchemaMessages/,
Expand All @@ -32,13 +33,11 @@ description = "Apache Beam :: SDKs :: Java :: Extensions :: Protobuf"
ext.summary = "Add support to Apache Beam for Google Protobuf."

dependencies {
compile library.java.vendored_guava_26_0_jre
compile project(path: ":sdks:java:core", configuration: "shadow")
compile library.java.vendored_guava_26_0_jre
compile library.java.protobuf_java
compile library.java.vendored_bytebuddy_1_10_8
testCompile project(path: ":sdks:java:core", configuration: "shadowTest")
testCompile library.java.hamcrest_core
testCompile library.java.hamcrest_library
testCompile library.java.mockito_core
testCompile library.java.junit
testRuntimeOnly library.java.slf4j_jdk14
}
12 changes: 8 additions & 4 deletions sdks/java/extensions/schemaio-expansion-service/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,15 @@ applyJavaNature(

dependencies {
compile project(path: ":sdks:java:expansion-service")
permitUnusedDeclared project(path: ":sdks:java:expansion-service")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are likely candidates for removal. We may even want to make a JIRA specifically for permitUnusedDeclared instances in *-expansion-service, because the solution for all these expansion services will be the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibzib By *-expansion-service, did you mean all these

./sdks/java/io/kinesis/expansion-service
./sdks/java/io/expansion-service
./sdks/java/io/snowflake/expansion-service
./sdks/java/io/google-cloud-platform/expansion-service
./sdks/java/expansion-service
./sdks/java/extensions/schemaio-expansion-service
./sdks/java/extensions/sql/expansion-service
./sdks/java/testing/expansion-service

I can file the JIRA as a sub-task of BEAM-11761.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we already merged some of these without a new JIRA. I don't think a new JIRA is strictly necessary - it's easy enough to find these instances if you're looking for them. So let's just use BEAM-11761.

compile project(":sdks:java:io:jdbc")
permitUnusedDeclared project(":sdks:java:io:jdbc")
compile library.java.postgres
permitUnusedDeclared library.java.postgres
compile project(path: ":model:pipeline", configuration: "shadow")
compile project(path: ":sdks:java:core", configuration: "shadow")
compile library.java.vendored_grpc_1_26_0
compile library.java.vendored_guava_26_0_jre
testCompile library.java.junit
testCompile library.java.powermock_mockito
testCompile library.java.mockito_core
// TODO(BEAM-10632): remove this dependency
testCompile "org.checkerframework:checker-qual:3.5.0"
}
}
6 changes: 3 additions & 3 deletions sdks/java/extensions/sketching/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

plugins { id 'org.apache.beam.module' }
applyJavaNature(
enableStrictDependencies: true,
automaticModuleName: 'org.apache.beam.sdk.extensions.sketching')

description = "Apache Beam :: SDKs :: Java :: Extensions :: Sketching"
Expand All @@ -26,15 +27,14 @@ def streamlib_version = "2.9.5"
def tdigest_version = "3.2"

dependencies {
compile library.java.slf4j_api
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this one is actually safe to remove. slf4j is used for logging, and this project doesn't contain any loggers. If anyone wants to add logging to this project, it's easy to add this dependency back in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibzib I'm curious how to determine whether this project contains loggers, and why removing this particular dependency entirely seems safer than removing others (where we are adding permitUnusedDeclared).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

permitUnusedDeclared library.java.slf4j_api
compile library.java.vendored_guava_26_0_jre
compile project(path: ":sdks:java:core", configuration: "shadow")
compile "com.clearspring.analytics:stream:$streamlib_version"
compile "com.tdunning:t-digest:$tdigest_version"
compile library.java.slf4j_api
testCompile library.java.avro
testCompile project(path: ":sdks:java:core", configuration: "shadowTest")
testCompile library.java.hamcrest_core
testCompile library.java.hamcrest_library
testCompile library.java.junit
testRuntimeOnly project(path: ":runners:direct-java", configuration: "shadow")
}
4 changes: 2 additions & 2 deletions sdks/java/extensions/sorter/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import java.util.stream.Collectors

plugins { id 'org.apache.beam.module' }
applyJavaNature(
enableStrictDependencies: true,
automaticModuleName: 'org.apache.beam.sdk.extensions.sorter')

description = "Apache Beam :: SDKs :: Java :: Extensions :: Sorter"
Expand All @@ -36,10 +37,9 @@ hadoopVersions.each {kv -> configurations.create("hadoopVersion$kv.key")}
dependencies {
compile project(path: ":sdks:java:core", configuration: "shadow")
compile library.java.vendored_guava_26_0_jre
compile library.java.slf4j_api
provided library.java.hadoop_mapreduce_client_core
provided library.java.hadoop_common
testCompile library.java.hamcrest_core
testCompile library.java.hamcrest_library
testCompile library.java.mockito_core
testCompile library.java.junit
testRuntimeOnly project(path: ":runners:direct-java", configuration: "shadow")
Expand Down
2 changes: 2 additions & 0 deletions sdks/java/extensions/zetasketch/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import groovy.json.JsonOutput

plugins { id 'org.apache.beam.module' }
applyJavaNature(
enableStrictDependencies: true,
automaticModuleName: 'org.apache.beam.sdk.extensions.zetasketch')

description = "Apache Beam :: SDKs :: Java :: Extensions :: ZetaSketch"
Expand All @@ -32,6 +33,7 @@ dependencies {
compile library.java.slf4j_api
compile library.java.vendored_guava_26_0_jre
compile project(path: ":sdks:java:core", configuration: "shadow")
compile "com.google.auto.value:auto-value-annotations:1.6.3"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? Aren't we already importing autovalue in BeamModulePlugin?

beam/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy

Line 870 in 705649d

"com.google.auto.value:auto-value-annotations:$autovalue_version",

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same kind of thing as the comment i added for sdks/java/extensions/ml/build.gradle. Here there is a runtime dependency on "com.google.auto.value:auto-value-annotations:1.6.3" via "com.google.zetasketch:zetasketch:0.1.0"

\--- com.google.zetasketch:zetasketch:0.1.0
     +--- com.google.auto.value:auto-value-annotations:1.6.3
     +--- com.google.code.findbugs:jsr305:3.0.2
     +--- com.google.errorprone:error_prone_annotations:2.3.2 -> 2.3.1
     +--- it.unimi.dsi:fastutil:8.2.2
     \--- org.checkerframework:checker-qual:2.8.1 -> 3.7.0

compile "com.google.zetasketch:zetasketch:$zetasketch_version"
testCompile library.java.junit
testCompile project(":sdks:java:io:google-cloud-platform")
Expand Down