kennknowles commented on a change in pull request #12938:
URL: https://github.com/apache/beam/pull/12938#discussion_r516882760
##########
File path:
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
##########
@@ -21,27 +21,22 @@ package org.apache.beam.gradle
import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar
import groovy.json.JsonOutput
import groovy.json.JsonSlurper
-import org.gradle.api.attributes.Category
import org.gradle.api.GradleException
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.artifacts.Configuration
import org.gradle.api.artifacts.ProjectDependency
+import org.gradle.api.attributes.Category
import org.gradle.api.file.FileCollection
import org.gradle.api.file.FileTree
import org.gradle.api.plugins.quality.Checkstyle
import org.gradle.api.publish.maven.MavenPublication
-import org.gradle.api.tasks.Copy
-import org.gradle.api.tasks.Delete
-import org.gradle.api.tasks.Exec
-import org.gradle.api.tasks.JavaExec
+import org.gradle.api.tasks.*
Review comment:
We prefer to not use `*` imports
##########
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:
These seem wrong. The direct runner should either use these or not
depend on them.
##########
File path:
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
##########
@@ -95,7 +90,7 @@ class BeamModulePlugin implements Plugin<Project> {
boolean ignoreRawtypeErrors = false
/** Controls whether the dependency analysis plugin is enabled. */
- boolean enableStrictDependencies = false
+ boolean enableStrictDependencies = true
Review comment:
Great. Once it is enabled we can delete the option, too.
##########
File path: examples/java/build.gradle
##########
@@ -67,7 +67,11 @@ dependencies {
compile library.java.joda_time
compile library.java.proto_google_cloud_datastore_v1
compile library.java.slf4j_api
- compile library.java.slf4j_jdk14
+ compile "com.google.api.grpc:proto-google-cloud-language-v1:1.81.4"
+ compile "com.google.code.findbugs:jsr305:3.0.2"
+ compile "com.google.oauth-client:google-oauth-client:1.31.0"
+ permitUnusedDeclared "com.google.auto.value:auto-value-annotations:1.7.2"
+ permitUnusedDeclared "org.checkerframework:checker-qual:3.7.0"
Review comment:
We prefer to use references to `library.auto_value_annotations`, etc.
You may have to add some.
##########
File path: runners/flink/1.10/build.gradle
##########
@@ -31,3 +38,19 @@ project.ext {
// Load the main build script which contains all build logic.
apply from: "$basePath/flink_runner.gradle"
+
+dependencies {
+ permitUnusedDeclared
"com.fasterxml.jackson.core:jackson-annotations:$jackson_annotations_version"
+ permitUnusedDeclared
"org.apache.flink:flink-clients_2.11:$flink_clients_version"
+ compile
"com.fasterxml.jackson.core:jackson-databind:$jackson_databind_verisioon"
+ compile "com.google.code.findbugs:jsr305:$jsr305_version"
+ compile "org.apache.flink:flink-annotations:$flink_annotations_version"
+ compile "org.apache.flink:flink-optimizer_2.11:$flink_optimizer_version"
+ compile "org.checkerframework:checker-qual:$checker_qual_version"
+ compile project(path: ":model:fn-execution", configuration: "shadow")
+ compile project(path: ":model:pipeline", configuration: "shadow")
+ compile project(path: ":model:job-management:", configuration: "shadow")
+ compile project(":sdks:java:fn-execution")
+ compile project(path: ":vendor:sdks-java-extensions-protobuf",
configuration: "shadow")
+ permitUnusedDeclared "org.checkerframework:checker-qual:3.7.0"
Review comment:
There should not be an unused declared dep but also an added 2.0.0 dep.
##########
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:
This version should match the same version as all the other modules
unless there is a reason to diverge. It is already added to every Java module
by `BeamModulePlugin`.
##########
File path:
buildSrc/src/main/groovy/org/apache/beam/gradle/GrpcVendoring_1_26_0.groovy
##########
@@ -26,57 +26,14 @@ class GrpcVendoring_1_26_0 {
static def guava_version = "26.0-jre"
static def protobuf_version = "3.11.0"
static def grpc_version = "1.26.0"
- static def gson_version = "2.8.6"
Review comment:
I don't understand these deletions
##########
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"
Review comment:
If the version is different between of Flink 1.10 then it could be
listed here, but needs a comment to explain 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:
[email protected]
