Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions sdks/java/io/hcatalog/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -53,24 +53,40 @@ dependencies {
provided library.java.jackson_databind
// Calcite (a dependency of Hive) bundles without repackaging Guava which is why we redeclare it
// here so that it appears on the compile/test/runtime classpath before Calcite.
provided library.java.joda_time
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The joda_time library is already declared as an implementation dependency on line 48. Adding it again as a provided dependency on line 56 might be redundant unless there's a specific reason related to dependency resolution order or classpath management for the provided scope that is not immediately apparent. If the implementation declaration already covers the need for joda_time during compilation, this line can be removed to avoid redundancy.

provided library.java.hadoop_common
provided "org.apache.hive:hive-exec:$hive_version"
provided(group: "org.apache.hive", name: "hive-exec", version: hive_version, classifier: "core")
provided(group: "org.apache.hive.hcatalog", name: "hive-hcatalog-core", version: hive_version) {
exclude group: "org.apache.hive", module: "hive-exec"
exclude group: "com.google.protobuf", module: "protobuf-java"
}
testImplementation library.java.joda_time
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the provided declaration, joda_time is already an implementation dependency on line 48. If the implementation dependency is sufficient for test compilation and runtime, this testImplementation declaration might be redundant. Please confirm if this explicit testImplementation is necessary, perhaps due to specific test classpath requirements or to override a transitive dependency.

testImplementation library.java.commons_io
testImplementation library.java.junit
testImplementation library.java.hamcrest
testImplementation "org.apache.hive.hcatalog:hive-hcatalog-core:$hive_version:tests"
testImplementation "org.apache.hive:hive-exec:$hive_version"
testImplementation("org.apache.hive.hcatalog:hive-hcatalog-core:$hive_version:tests") {
exclude group: "org.apache.hive", module: "hive-exec"
}
testImplementation(group: "org.apache.hive", name: "hive-exec", version: hive_version)
testImplementation("org.apache.hive:hive-standalone-metastore-common:$hive_version")
testImplementation("org.apache.hive:hive-standalone-metastore-server:$hive_version")
testImplementation("org.apache.calcite:calcite-core:1.25.0")
testImplementation("org.apache.datasketches:datasketches-hive:1.1.0-incubating")
// datanucleus dependency version should be in alignment with managed dependencies of hive-standalone-metastore
testRuntimeOnly 'org.datanucleus:datanucleus-api-jdo:5.2.8'
testRuntimeOnly 'org.datanucleus:datanucleus-rdbms:5.2.10'
testRuntimeOnly 'org.datanucleus:javax.jdo:3.2.0-release'
testImplementation "org.apache.hive:hive-common:$hive_version"
testImplementation "org.apache.hive:hive-cli:$hive_version"
testImplementation "org.apache.hive.hcatalog:hive-hcatalog-core:$hive_version"
testImplementation("org.apache.hive:hive-common:$hive_version") {
exclude group: "joda-time", module: "joda-time"
exclude group: "org.apache.hive", module: "hive-exec"
}
testImplementation("org.apache.hive:hive-cli:$hive_version") {
exclude group: "joda-time", module: "joda-time"
exclude group: "org.apache.hive", module: "hive-exec"
}
testImplementation("org.apache.hive.hcatalog:hive-hcatalog-core:$hive_version") {
exclude group: "joda-time", module: "joda-time"
exclude group: "org.apache.hive", module: "hive-exec"
}
testImplementation project(path: ":sdks:java:io:common")
testRuntimeOnly project(path: ":runners:direct-java", configuration: "shadow")
hadoopVersions.each {kv ->
Expand Down Expand Up @@ -107,5 +123,10 @@ hadoopVersions.each { kv ->
description = "Runs HCatalog tests with Hadoop version $kv.value"
classpath = configurations."hadoopVersion$kv.key" + sourceSets.test.runtimeClasspath
include '**/*Test.class'
jvmArgs "--add-opens=java.base/java.net=ALL-UNNAMED"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The jvmArgs for --add-opens is already applied globally to all Test tasks via the tasks.withType(Test).configureEach block starting on line 130. This specific declaration on line 126 for hadoopVersion${kv.key}Test tasks becomes redundant and can be removed.

}
}

tasks.withType(Test).configureEach {
jvmArgs "--add-opens=java.base/java.net=ALL-UNNAMED"
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ The 1.2 jars provide the necessary methods for Beam while remain compatible with
<groupId>org.apache.hive</groupId>
<artifactId>hive-exec</artifactId>
<version>1.2</version>
<classifier>core</classifier>
</dependency>
<dependency>
<groupId>org.apache.hive</groupId>
Expand Down
Loading