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



##########
File path: sdks/java/extensions/sql/datacatalog/build.gradle
##########
@@ -20,13 +20,23 @@ import groovy.json.JsonOutput
 
 plugins { id 'org.apache.beam.module' }
 
-applyJavaNature(automaticModuleName: 
'org.apache.beam.sdk.extensions.sql.datacatalog')
+applyJavaNature(
+  enableStrictDependencies: true,
+  automaticModuleName: 'org.apache.beam.sdk.extensions.sql.datacatalog')
 
 dependencies {
   compile enforcedPlatform(library.java.google_cloud_platform_libraries_bom)
   compile(library.java.google_cloud_datacatalog_v1beta1) {
     exclude group: 'io.grpc', module: 'grpc-core' // Use Beam's version
   }
+  compile library.java.gax
+  compile library.java.google_auth_library_credentials
+  compile library.java.protobuf_java
+  compile library.java.slf4j_api
+  compile library.java.vendored_guava_26_0_jre
+  compile project(path: ":sdks:java:core", configuration: "shadow")
+  compile "com.google.api.grpc:proto-google-cloud-datacatalog-v1beta1:0.32.1"

Review comment:
       We should probably used the bom for this, like we discussed on the PR 
for GCP IO.

##########
File path: sdks/java/extensions/sql/datacatalog/build.gradle
##########
@@ -35,6 +45,8 @@ dependencies {
 
   testCompile project(":sdks:java:extensions:sql:zetasql")
   testRuntimeOnly library.java.slf4j_simple
+  permitUnusedDeclared "org.threeten:threetenbp:1.4.5"

Review comment:
       Nit: please put the `permitUnusedDeclared` lines right under where they 
are declared, so it's easier to read. e.g. 
   
   ```
   compile "org.threeten:threetenbp:1.4.5"
   permitUnusedDeclared "org.threeten:threetenbp:1.4.5"
   ```

##########
File path: sdks/java/extensions/sql/zetasql/build.gradle
##########
@@ -31,25 +32,23 @@ 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")
   compile project(":sdks:java:extensions:sql")
   compile library.java.vendored_calcite_1_20_0
   compile library.java.guava
   compile library.java.grpc_api
   compile library.java.protobuf_java
-  compile library.java.protobuf_java_util
-  compile "com.google.api.grpc:proto-google-common-protos:1.12.0" // 
Interfaces with ZetaSQL use this
-  compile "com.google.api.grpc:grpc-google-common-protos:1.12.0" // Interfaces 
with ZetaSQL use this
-  compile "com.google.zetasql:zetasql-jni-channel:$zetasql_version"
   compile "com.google.zetasql:zetasql-client:$zetasql_version"
   compile "com.google.zetasql:zetasql-types:$zetasql_version"
+  compile library.java.joda_time
+  compile library.java.vendored_guava_26_0_jre
+  permitUnusedDeclared library.java.protobuf_java_util
+  compile "com.google.zetasql:zetasql-jni-channel:$zetasql_version"

Review comment:
       I'm guessing this should be runtimeOnly instead, but leaving it as 
compile + permitUnusedDeclared is safer.

##########
File path: sdks/java/extensions/sql/build.gradle
##########
@@ -62,7 +63,6 @@ dependencies {
   fmppTask "org.freemarker:freemarker:2.3.28"
   fmppTemplates library.java.vendored_calcite_1_20_0
   compile project(":sdks:java:core")

Review comment:
       I wonder if the issue here is that :sdks:java:core should use the 
"shadow" configuration, like elsewhere.
   
   ```suggestion
     compile project(":sdks:java:core", configuration: "shadow")
   ```

##########
File path: sdks/java/extensions/sql/zetasql/build.gradle
##########
@@ -31,25 +32,23 @@ 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")
   compile project(":sdks:java:extensions:sql")
   compile library.java.vendored_calcite_1_20_0
   compile library.java.guava
   compile library.java.grpc_api
   compile library.java.protobuf_java
-  compile library.java.protobuf_java_util
-  compile "com.google.api.grpc:proto-google-common-protos:1.12.0" // 
Interfaces with ZetaSQL use this
-  compile "com.google.api.grpc:grpc-google-common-protos:1.12.0" // Interfaces 
with ZetaSQL use this
-  compile "com.google.zetasql:zetasql-jni-channel:$zetasql_version"
   compile "com.google.zetasql:zetasql-client:$zetasql_version"
   compile "com.google.zetasql:zetasql-types:$zetasql_version"
+  compile library.java.joda_time
+  compile library.java.vendored_guava_26_0_jre
+  permitUnusedDeclared library.java.protobuf_java_util

Review comment:
       Since you removed library.java.protobuf_java_util, you shouldn't need to 
`permitUnusedDeclared` any more.

##########
File path: sdks/java/extensions/sql/expansion-service/build.gradle
##########
@@ -34,7 +35,8 @@ ext.summary = """Contains code to run a SQL Expansion 
Service."""
 dependencies {
   compile project(path: ":sdks:java:extensions:sql")
   compile project(path: ":sdks:java:extensions:sql:zetasql")
-  compile project(path: ":sdks:java:expansion-service")
+  compile project(path: ":sdks:java:core", configuration: "shadow")
+  compile "org.apache.beam:beam-vendor-guava-26_0-jre:0.1"

Review comment:
       ```suggestion
     compile library.java.vendored_guava_26_0_jre
   ```

##########
File path: sdks/java/extensions/sql/hcatalog/build.gradle
##########
@@ -38,6 +39,5 @@ dependencies {
   testCompile("org.apache.hive.hcatalog:hive-hcatalog-core:$hive_version") {
     // Hive brings full Calcite 1.6 + Avatica with JDBC driver which
     // gets registered and gets started instead of ours
-    exclude group: "org.apache.calcite", module:"calcite-avatica"

Review comment:
       Why should this line be removed?

##########
File path: sdks/java/extensions/sql/jdbc/build.gradle
##########
@@ -32,18 +33,16 @@ configurations {
 
 dependencies {
   compile project(":sdks:java:extensions:sql")
-  compile "jline:jline:2.14.6"

Review comment:
       This might need to be runtimeOnly instead.

##########
File path: sdks/java/extensions/sql/jdbc/build.gradle
##########
@@ -32,18 +33,16 @@ configurations {
 
 dependencies {
   compile project(":sdks:java:extensions:sql")
-  compile "jline:jline:2.14.6"
   compile "sqlline:sqlline:1.4.0"
-  compile library.java.slf4j_jdk14

Review comment:
       This should probably be runtimeOnly 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:
[email protected]


Reply via email to