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]