kennknowles commented on a change in pull request #12938:
URL: https://github.com/apache/beam/pull/12938#discussion_r529728646
##########
File path:
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
##########
@@ -850,7 +849,7 @@ class BeamModulePlugin implements Plugin<Project> {
// configurations because they are never required to be shaded or become
a
// dependency of the output.
def compileOnlyAnnotationDeps = [
- "com.google.auto.value:auto-value-annotations:1.7",
+ "com.google.auto.value:auto-value-annotations:1.7.2",
Review comment:
Around here, you can add these dependencies to `permitUnusedDeclared` so
you don't have to add it to all modules.
##########
File path:
buildSrc/src/main/groovy/org/apache/beam/gradle/GrpcVendoring_1_26_0.groovy
##########
@@ -43,40 +43,13 @@ class GrpcVendoring_1_26_0 {
static def npn_api_version = "1.1.1.v20141010"
static def jboss_marshalling_version = "1.4.11.Final"
static def jboss_modules_version = "1.1.0.Beta1"
-
/** Returns the list of compile time dependencies. */
static List<String> dependencies() {
return [
- "com.google.guava:guava:$guava_version",
Review comment:
The change here concerns me - does it introduce linkage errors?
##########
File path: sdks/java/extensions/sql/zetasql/build.gradle
##########
@@ -31,25 +31,24 @@ def zetasql_version = "2020.10.1"
dependencies {
compile enforcedPlatform(library.java.google_cloud_platform_libraries_bom)
- compile project(":sdks:java:core")
+ compile project(path: ":sdks:java:core", configuration: "shadow")
Review comment:
This is a major change.
CC @apilloud for whether it is right
##########
File path: sdks/java/extensions/sql/build.gradle
##########
@@ -71,7 +70,15 @@ dependencies {
compile "com.alibaba:fastjson:1.2.69"
compile "org.codehaus.janino:janino:3.0.11"
compile "org.codehaus.janino:commons-compiler:3.0.11"
- provided "org.checkerframework:checker-qual:3.4.1"
+ compile library.java.jackson_core
+ compile library.java.jsr305
+ compile library.java.vendored_guava_26_0_jre
+ compile "org.mongodb:mongo-java-driver:3.9.1"
+ compile "org.slf4j:slf4j-api:1.7.30"
+ compile "org.apache.beam:beam-vendor-guava-26_0-jre:0.1"
Review comment:
You already have `library.vendored_guava_26_0_jre` below. That should be
the only one.
##########
File path: runners/flink/1.10/build.gradle
##########
@@ -17,6 +17,13 @@
*/
def basePath = '..'
+def jackson_annotations_version = "2.10.2"
+def flink_clients_version = "1.10.1"
+def jackson_databind_verisioon = "2.10.2"
+def jsr305_version = "3.0.2"
+def flink_annotations_version = "1.10.1"
+def flink_optimizer_version = "1.10.1"
+def checker_qual_version = "2.0.0"
Review comment:
I meant my comment just on the checker qual version.
##########
File path: model/job-management/build.gradle
##########
@@ -32,4 +32,7 @@ dependencies {
// export the shaded variant as the actual runtime dependency.
compile project(path: ":model:pipeline", configuration: "unshaded")
runtime project(path: ":model:pipeline", configuration: "shadow")
+ permitUnusedDeclared library.java.vendored_grpc_1_26_0
+ permitUnusedDeclared "org.checkerframework:checker-qual:3.7.0"
+ compile "com.google.guava:guava:28.1-android"
Review comment:
This is interesting. If generated protos depend on this version of
Guava, we may need to do something about it.
##########
File path: sdks/java/extensions/sorter/build.gradle
##########
@@ -36,10 +36,10 @@ 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.jsr305
+ compile "org.slf4j:slf4j-api:1.7.30"
Review comment:
This should be `library.slf4j_api` and please set up the version in
`BeamModulePlugin.groovy`. The same for all libraries.
##########
File path: runners/flink/1.10/build.gradle
##########
@@ -31,3 +35,19 @@ project.ext {
// Load the main build script which contains all build logic.
apply from: "$basePath/flink_runner.gradle"
+
+dependencies {
+ permitUnusedDeclared library.java.jackson_annotations
Review comment:
The dependencies are set up in `flink_runner.gradle` using the versions
in the `project.ext` block.
@mxm may have some comment or recommend someone for a comment.
##########
File path: runners/flink/1.10/build.gradle
##########
@@ -17,6 +17,10 @@
*/
def basePath = '..'
+def flink_clients_version = "1.10.1"
+def flink_annotations_version = "1.10.1"
+def flink_optimizer_version = "1.10.1"
Review comment:
These flink versions are set up just below here in the `project.ext`
section.
@mxm may have some comment or recommend someone for a comment.
##########
File path: runners/flink/1.11/build.gradle
##########
@@ -31,3 +35,19 @@ project.ext {
// Load the main build script which contains all build logic.
apply from: "$basePath/flink_runner.gradle"
+
+dependencies {
+ permitUnusedDeclared library.java.jackson_annotations
+ permitUnusedDeclared
"org.apache.flink:flink-clients_2.11:$flink_clients_version"
Review comment:
I don't understand how the flink client is unused. Please add a comment
to explain each exclusion. We can try to ask people to understand.
##########
File path: runners/direct-java/build.gradle
##########
@@ -70,17 +71,16 @@ dependencies {
shadow library.java.vendored_grpc_1_26_0
shadow library.java.joda_time
shadow library.java.slf4j_api
- shadow library.java.args4j
+ permitUnusedDeclared "org.apache.beam:beam-vendor-grpc-1_26_0:0.3"
+ permitUnusedDeclared project(":runners:java-fn-execution")
+ permitUnusedDeclared project(":sdks:java:fn-execution")
Review comment:
I see. So these are really `testRuntime` dependencies. If we had a
`shadowTestRuntime` configuration we would exclude it from checking I guess.
Since we created the `shadowTest` configuration I don't think there is a
`shadowTestRuntime` configuration. Please add a comment explaining these
exclusions.
----------------------------------------------------------------
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:
[email protected]
