You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2021/02/11 19:04:10 UTC

[GitHub] [beam] ibzib commented on a change in pull request #13686: Enabled strict dependency on sdks-extensions

ibzib commented on a change in pull request #13686:
URL: https://github.com/apache/beam/pull/13686#discussion_r574742840



##########
File path: sdks/java/extensions/kryo/build.gradle
##########
@@ -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"

Review comment:
       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.

##########
File path: sdks/java/extensions/ml/build.gradle
##########
@@ -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")
     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"

Review comment:
       Why does this need to be permitUsedUndeclared? Don't we already declare it in BeamModulePlugin? https://github.com/apache/beam/blob/705649dc2f577e0fa4d8c766b89f7489d70acc60/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy#L870

##########
File path: sdks/java/extensions/sketching/build.gradle
##########
@@ -26,15 +27,14 @@ def streamlib_version = "2.9.5"
 def tdigest_version = "3.2"
 
 dependencies {
+  compile library.java.slf4j_api

Review comment:
       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.

##########
File path: sdks/java/extensions/schemaio-expansion-service/build.gradle
##########
@@ -31,11 +31,15 @@ applyJavaNature(
 
 dependencies {
     compile project(path: ":sdks:java:expansion-service")
+    permitUnusedDeclared project(path: ":sdks:java:expansion-service")

Review comment:
       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.

##########
File path: sdks/java/extensions/zetasketch/build.gradle
##########
@@ -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"

Review comment:
       Why is this needed? Aren't we already importing autovalue in BeamModulePlugin? https://github.com/apache/beam/blob/705649dc2f577e0fa4d8c766b89f7489d70acc60/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy#L870

##########
File path: sdks/java/extensions/ml/build.gradle
##########
@@ -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")

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




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