nastra commented on code in PR #7694:
URL: https://github.com/apache/iceberg/pull/7694#discussion_r1261135342


##########
settings.gradle:
##########
@@ -190,5 +190,4 @@ if (JavaVersion.current() == JavaVersion.VERSION_1_8) {
     project(':hive3').name = 'iceberg-hive3'
     project(':hive3-orc-bundle').name = 'iceberg-hive3-orc-bundle'
   }
-}
-
+}

Review Comment:
   nit: we can probably revert changes to this file



##########
gradle/libs.versions.toml:
##########
@@ -0,0 +1,153 @@
+# Also see settings.gradle file for dynamic versionCatalogs dependencies
+
+[versions]
+antlr = "4.9.3"
+deltaStandalone = "0.6.0"
+deltaCore = "2.2.0"
+flink115 = { strictly = "[1.15, 1.16[", prefer = "1.15.0"}
+flink116 = { strictly = "[1.16, 1.17[", prefer = "1.16.2"}
+flink117 = { strictly = "[1.17, 1.18[", prefer = "1.17.1"}
+guava = "32.1.1-jre"
+hadoop2 = "2.7.3"
+hive2 = { strictly = "[2, 3[", prefer = "2.3.9"}
+hive3 = "3.1.3"
+jackson211 = "2.11.4"
+jackson212 = "2.12.3"
+jackson213 = "2.13.4"
+jetty = "9.4.43.v20210629"
+junitJupiter = "5.9.2"
+mockito = "4.11.0"
+nessie = "0.63.0"
+orc = "1.9.0"
+parquet = "1.13.1"
+scalaCollectionCompat = "2.6.0"
+sparkHive31 = "3.1.3"
+sparkHive32 = "3.2.2"
+sparkHive33 = "3.3.2"
+sparkHive34 = "3.4.1"
+springBoot = "2.5.4"
+tez010 = "0.10.2"
+tez08 = { strictly = "[0.8, 0.9[", prefer = "0.8.4"}
+
+[libraries]
+activation = "javax.activation:activation:1.1.1"
+aircompressor = "io.airlift:aircompressor:0.24"
+aliyun-sdk-oss = "com.aliyun.oss:aliyun-sdk-oss:3.10.2"
+antlr-antlr4 = { module = "org.antlr:antlr4", version.ref = "antlr" }
+antlr-runtime = { module = "org.antlr:antlr4-runtime", version.ref = "antlr" }
+arrow-memory-netty = "org.apache.arrow:arrow-memory-netty:11.0.0"
+arrow-vector = "org.apache.arrow:arrow-vector:11.0.0"
+avro-avro = "org.apache.avro:avro:1.11.1"
+awssdk-bom = { module = "software.amazon.awssdk:bom", version = "2.20.18" }
+caffeine = "com.github.ben-manes.caffeine:caffeine:2.9.3"
+calcite-core = "org.apache.calcite:calcite-core:1.10.0"
+calcite-druid = "org.apache.calcite:calcite-druid:1.10.0"

Review Comment:
   wouldn't it make sense to extract this into a version and use it for both 
libs?



##########
gradle/libs.versions.toml:
##########
@@ -0,0 +1,153 @@
+# Also see settings.gradle file for dynamic versionCatalogs dependencies

Review Comment:
   does this comment still apply? I don't see anything in particular in 
`settings.gradle`



##########
gradle/libs.versions.toml:
##########
@@ -0,0 +1,153 @@
+# Also see settings.gradle file for dynamic versionCatalogs dependencies
+
+[versions]
+antlr = "4.9.3"
+deltaStandalone = "0.6.0"

Review Comment:
   to be consistent, maybe the version definitions should be in the form 
`delta-standalone` / `spark-hive-31` rather than `deltaStandalone` / 
`sparkHive31`



##########
build.gradle:
##########
@@ -344,33 +335,33 @@ project(':iceberg-core') {
     api project(':iceberg-api')
     implementation project(':iceberg-common')
     implementation project(path: ':iceberg-bundled-guava', configuration: 
'shadow')
-    annotationProcessor "org.immutables:value"
-    compileOnly "org.immutables:value"
+    annotationProcessor libs.immutables.value
+    compileOnly libs.immutables.value
 
-    implementation("org.apache.avro:avro") {
+    implementation(libs.avro.avro) {
       exclude group: 'org.tukaani' // xz compression is not supported
     }
 
-    implementation 'io.airlift:aircompressor'
-    implementation 'org.apache.httpcomponents.client5:httpclient5'
-    implementation "com.fasterxml.jackson.core:jackson-databind"
+    implementation libs.aircompressor
+    implementation libs.httpcomponents.httpclient5
+    implementation platform(libs.jackson.bom)

Review Comment:
   I think we should have a `jackson-databind = { module = 
"com.fasterxml.jackson.core:jackson-databind", version.ref = "jackson-ver" }` 
definition and then this would be using `implementation libs.jackson.databind`



##########
build.gradle:
##########
@@ -539,13 +536,14 @@ project(':iceberg-delta-lake') {
     implementation project(':iceberg-common')
     implementation project(':iceberg-core')
     implementation project(':iceberg-parquet')
+    implementation platform(libs.jackson.bom)
     implementation "com.fasterxml.jackson.core:jackson-databind"
-    annotationProcessor "org.immutables:value"
-    compileOnly "org.immutables:value"
+    annotationProcessor libs.immutables.value
+    compileOnly libs.immutables.value
 
-    compileOnly "io.delta:delta-standalone_${scalaVersion}"
+    compileOnly 
"io.delta:delta-standalone_${scalaVersion}:${libs.versions.deltaStandalone.get()}"

Review Comment:
   is there a better way to define this? Also why does this need a version now?



##########
build.gradle:
##########
@@ -555,17 +553,17 @@ project(':iceberg-delta-lake') {
     // The newest version of delta-core uses Spark 3.3.*. Since its only for 
test, we do
     // not need to include older version of delta-core
     if (sparkVersions.contains("3.3")) {
-      integrationImplementation "io.delta:delta-core_${scalaVersion}"
+      integrationImplementation 
"io.delta:delta-core_${scalaVersion}:${libs.versions.deltaCore.get()}"
       integrationImplementation project(path: 
":iceberg-spark:iceberg-spark-3.3_${scalaVersion}")
-      integrationImplementation("org.apache.hadoop:hadoop-minicluster") {
+      integrationImplementation(libs.hadoop2.minicluster) {
         exclude group: 'org.apache.avro', module: 'avro'
         // to make sure netty libs only come from project(':iceberg-arrow')
         exclude group: 'io.netty', module: 'netty-buffer'
         exclude group: 'io.netty', module: 'netty-common'
       }
       integrationImplementation project(path: ':iceberg-hive-metastore')
       integrationImplementation project(path: ':iceberg-hive-metastore', 
configuration: 'testArtifacts')
-      
integrationImplementation("org.apache.spark:spark-hive_${scalaVersion}:3.3.2") {
+      integrationImplementation(libs.spark.hive33) {

Review Comment:
   why don't we need a scalaversion here but require one for 
`io.delta:delta-standalone_${scalaVersion}`?



##########
gradle/libs.versions.toml:
##########
@@ -0,0 +1,153 @@
+# Also see settings.gradle file for dynamic versionCatalogs dependencies
+
+[versions]
+antlr = "4.9.3"
+deltaStandalone = "0.6.0"
+deltaCore = "2.2.0"
+flink115 = { strictly = "[1.15, 1.16[", prefer = "1.15.0"}
+flink116 = { strictly = "[1.16, 1.17[", prefer = "1.16.2"}
+flink117 = { strictly = "[1.17, 1.18[", prefer = "1.17.1"}
+guava = "32.1.1-jre"
+hadoop2 = "2.7.3"
+hive2 = { strictly = "[2, 3[", prefer = "2.3.9"}
+hive3 = "3.1.3"
+jackson211 = "2.11.4"
+jackson212 = "2.12.3"
+jackson213 = "2.13.4"
+jetty = "9.4.43.v20210629"
+junitJupiter = "5.9.2"
+mockito = "4.11.0"
+nessie = "0.63.0"
+orc = "1.9.0"
+parquet = "1.13.1"
+scalaCollectionCompat = "2.6.0"
+sparkHive31 = "3.1.3"
+sparkHive32 = "3.2.2"
+sparkHive33 = "3.3.2"
+sparkHive34 = "3.4.1"
+springBoot = "2.5.4"
+tez010 = "0.10.2"
+tez08 = { strictly = "[0.8, 0.9[", prefer = "0.8.4"}
+
+[libraries]
+activation = "javax.activation:activation:1.1.1"
+aircompressor = "io.airlift:aircompressor:0.24"
+aliyun-sdk-oss = "com.aliyun.oss:aliyun-sdk-oss:3.10.2"
+antlr-antlr4 = { module = "org.antlr:antlr4", version.ref = "antlr" }
+antlr-runtime = { module = "org.antlr:antlr4-runtime", version.ref = "antlr" }
+arrow-memory-netty = "org.apache.arrow:arrow-memory-netty:11.0.0"
+arrow-vector = "org.apache.arrow:arrow-vector:11.0.0"
+avro-avro = "org.apache.avro:avro:1.11.1"
+awssdk-bom = { module = "software.amazon.awssdk:bom", version = "2.20.18" }
+caffeine = "com.github.ben-manes.caffeine:caffeine:2.9.3"
+calcite-core = "org.apache.calcite:calcite-core:1.10.0"
+calcite-druid = "org.apache.calcite:calcite-druid:1.10.0"
+delta-standalone = { module = "io.delta:delta-standalone_2.12", version.ref = 
"deltaStandalone" }
+errorprone-annotations = "com.google.errorprone:error_prone_annotations:2.3.3"
+findbugs-jsr305 = "com.google.code.findbugs:jsr305:3.0.2"
+flink115-avro = { module = "org.apache.flink:flink-avro", version.ref = 
"flink115" }
+flink115-connector-base = { module = "org.apache.flink:flink-connector-base", 
version.ref = "flink115" }
+flink115-connector-files = { module = 
"org.apache.flink:flink-connector-files", version.ref = "flink115" }
+flink115-metrics-dropwizard = { module = 
"org.apache.flink:flink-metrics-dropwizard", version.ref = "flink115" }
+flink115-streaming-java = { module = "org.apache.flink:flink-streaming-java", 
version.ref = "flink115" }
+flink115-table-api-java-bridge = { module = 
"org.apache.flink:flink-table-api-java-bridge", version.ref = "flink115" }
+flink116-avro = { module = "org.apache.flink:flink-avro", version.ref = 
"flink116" }
+flink116-connector-base = { module = "org.apache.flink:flink-connector-base", 
version.ref = "flink116" }
+flink116-connector-files = { module = 
"org.apache.flink:flink-connector-files", version.ref = "flink116" }
+flink116-metrics-dropwizard = { module = 
"org.apache.flink:flink-metrics-dropwizard", version.ref = "flink116" }
+flink116-streaming-java = { module = "org.apache.flink:flink-streaming-java", 
version.ref = "flink116" }
+flink116-table-api-java-bridge = { module = 
"org.apache.flink:flink-table-api-java-bridge", version.ref = "flink116" }
+flink117-avro = { module = "org.apache.flink:flink-avro", version.ref = 
"flink117" }
+flink117-connector-base = { module = "org.apache.flink:flink-connector-base", 
version.ref = "flink117" }
+flink117-connector-files = { module = 
"org.apache.flink:flink-connector-files", version.ref = "flink117" }
+flink117-metrics-dropwizard = { module = 
"org.apache.flink:flink-metrics-dropwizard", version.ref = "flink117" }
+flink117-streaming-java = { module = "org.apache.flink:flink-streaming-java", 
version.ref = "flink117" }
+flink117-table-api-java-bridge = { module = 
"org.apache.flink:flink-table-api-java-bridge", version.ref = "flink117" }
+google-libraries-bom = "com.google.cloud:libraries-bom:24.1.0"
+guava-guava = { module = "com.google.guava:guava", version.ref = "guava" }
+hadoop2-client = { module = "org.apache.hadoop:hadoop-client", version.ref = 
"hadoop2" }
+hadoop2-common = { module = "org.apache.hadoop:hadoop-common", version.ref = 
"hadoop2" }
+hadoop2-hdfs = { module = "org.apache.hadoop:hadoop-hdfs", version.ref = 
"hadoop2" }
+hadoop2-mapreduce-client-core = { module = 
"org.apache.hadoop:hadoop-mapreduce-client-core", version.ref = "hadoop2" }
+hadoop2-minicluster = { module = "org.apache.hadoop:hadoop-minicluster", 
version.ref = "hadoop2" }
+hadoop3-client = "org.apache.hadoop:hadoop-client:3.3.6"
+hive2-exec = { module = "org.apache.hive:hive-exec", version.ref = "hive2" }
+hive2-metastore = { module = "org.apache.hive:hive-metastore", version.ref = 
"hive2" }
+hive2-serde = { module = "org.apache.hive:hive-serde", version.ref = "hive2" }
+hive2-service = { module = "org.apache.hive:hive-service", version.ref = 
"hive2" }
+hive3-exec = { module = "org.apache.hive:hive-exec", version.ref = "hive3" }
+hive3-metastore = { module = "org.apache.hive:hive-metastore", version.ref = 
"hive3" }
+hive3-serde = { module = "org.apache.hive:hive-serde", version.ref = "hive3" }
+hive3-service = { module = "org.apache.hive:hive-service", version.ref = 
"hive3" }
+httpcomponents-httpclient5 = 
"org.apache.httpcomponents.client5:httpclient5:5.2.1"
+immutables-value = "org.immutables:value:2.9.2"
+jackson-bom = { module = "com.fasterxml.jackson:jackson-bom", version = 
"2.14.2" }
+jackson211-bom = { module = "com.fasterxml.jackson:jackson-bom", version = { 
strictly = "[2.11, 2.12[", prefer = "2.11.4"} }
+jackson212-bom = { module = "com.fasterxml.jackson:jackson-bom", version = { 
strictly = "[2.12, 2.13[", prefer = "2.12.3"} }
+jackson213-bom = { module = "com.fasterxml.jackson:jackson-bom", version =  { 
strictly = "[2.13, 2.14[", prefer = "2.13.4"} }

Review Comment:
   I believe it's better to generally specify the particular version to be used 
in this whole file rather than having version ranges



##########
spark/v3.1/build.gradle:
##########
@@ -17,24 +17,20 @@
  * under the License.
  */
 
+String scalaVersion = System.getProperty("scalaVersion") != null ? 
System.getProperty("scalaVersion") : System.getProperty("defaultScalaVersion")
+
 def sparkProjects = [
     project(':iceberg-spark:iceberg-spark-3.1_2.12'),
     project(":iceberg-spark:iceberg-spark-extensions-3.1_2.12"),
     project(':iceberg-spark:iceberg-spark-runtime-3.1_2.12')
 ]
 
 configure(sparkProjects) {
-  project.ext {
-    sparkVersion = '3.1.3'
-  }
 
   configurations {
     all {
       resolutionStrategy {
-        force 'com.fasterxml.jackson.module:jackson-module-scala_2.12:2.11.4'
-        force 'com.fasterxml.jackson.module:jackson-module-paranamer:2.11.4'
-        force 'com.fasterxml.jackson.core:jackson-core:2.11.4'
-        force 'com.fasterxml.jackson.core:jackson-databind:2.11.4'
+        force 
"com.fasterxml.jackson.module:jackson-module-scala_${scalaVersion}:${libs.versions.jackson211.get()}"

Review Comment:
   what about forcing the other versions? I think integration tests might fail 
if those versions aren't aligning



##########
gradle/libs.versions.toml:
##########
@@ -0,0 +1,153 @@
+# Also see settings.gradle file for dynamic versionCatalogs dependencies
+
+[versions]
+antlr = "4.9.3"
+deltaStandalone = "0.6.0"
+deltaCore = "2.2.0"
+flink115 = { strictly = "[1.15, 1.16[", prefer = "1.15.0"}
+flink116 = { strictly = "[1.16, 1.17[", prefer = "1.16.2"}
+flink117 = { strictly = "[1.17, 1.18[", prefer = "1.17.1"}

Review Comment:
   I think we should be using a very specific flink version for 1.15 / 1.16 / 
1.17 as I believe there have been breaking changes in the past between patch 
versions



##########
spark/v3.2/build.gradle:
##########
@@ -31,10 +30,7 @@ configure(sparkProjects) {
   configurations {
     all {
       resolutionStrategy {
-        force 
"com.fasterxml.jackson.module:jackson-module-scala_${scalaVersion}:2.12.3"
-        force 'com.fasterxml.jackson.module:jackson-module-paranamer:2.12.3'
-        force 'com.fasterxml.jackson.core:jackson-databind:2.12.3'
-        force 'com.fasterxml.jackson.core:jackson-core:2.12.3'
+        force 
"com.fasterxml.jackson.module:jackson-module-scala_${scalaVersion}:${libs.versions.jackson212.get()}"

Review Comment:
   same question as for Spark 3.1



##########
spark/v3.2/build.gradle:
##########
@@ -60,11 +56,11 @@ 
project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}") {
     implementation project(':iceberg-orc')
     implementation project(':iceberg-parquet')
     implementation project(':iceberg-arrow')
-    
implementation("org.scala-lang.modules:scala-collection-compat_${scalaVersion}")
+    implementation 
"org.scala-lang.modules:scala-collection-compat_${scalaVersion}:2.6.0"

Review Comment:
   or should this be using `scalaCollectionCompat`?



##########
build.gradle:
##########
@@ -461,46 +455,49 @@ project(':iceberg-aws') {
     api project(':iceberg-api')
     implementation project(':iceberg-common')
     implementation project(':iceberg-core')
-    annotationProcessor "org.immutables:value"
-    compileOnly "org.immutables:value"
-    implementation "com.github.ben-manes.caffeine:caffeine"
+    annotationProcessor libs.immutables.value
+    compileOnly libs.immutables.value
+    implementation libs.caffeine
+    implementation platform(libs.jackson.bom)

Review Comment:
   same as above



##########
spark/v3.1/build.gradle:
##########
@@ -131,20 +131,21 @@ 
project(":iceberg-spark:iceberg-spark-extensions-3.1_2.12") {
   }
 
   dependencies {
-    compileOnly "org.scala-lang:scala-library"
+    compileOnly "org.scala-lang:scala-library:${scalaVersion}"

Review Comment:
   why do we need to set this now?



##########
gradle/libs.versions.toml:
##########
@@ -0,0 +1,153 @@
+# Also see settings.gradle file for dynamic versionCatalogs dependencies
+
+[versions]
+antlr = "4.9.3"
+deltaStandalone = "0.6.0"
+deltaCore = "2.2.0"
+flink115 = { strictly = "[1.15, 1.16[", prefer = "1.15.0"}
+flink116 = { strictly = "[1.16, 1.17[", prefer = "1.16.2"}
+flink117 = { strictly = "[1.17, 1.18[", prefer = "1.17.1"}
+guava = "32.1.1-jre"
+hadoop2 = "2.7.3"
+hive2 = { strictly = "[2, 3[", prefer = "2.3.9"}
+hive3 = "3.1.3"
+jackson211 = "2.11.4"
+jackson212 = "2.12.3"
+jackson213 = "2.13.4"
+jetty = "9.4.43.v20210629"
+junitJupiter = "5.9.2"
+mockito = "4.11.0"
+nessie = "0.63.0"
+orc = "1.9.0"
+parquet = "1.13.1"
+scalaCollectionCompat = "2.6.0"
+sparkHive31 = "3.1.3"
+sparkHive32 = "3.2.2"
+sparkHive33 = "3.3.2"
+sparkHive34 = "3.4.1"
+springBoot = "2.5.4"
+tez010 = "0.10.2"
+tez08 = { strictly = "[0.8, 0.9[", prefer = "0.8.4"}
+
+[libraries]
+activation = "javax.activation:activation:1.1.1"
+aircompressor = "io.airlift:aircompressor:0.24"
+aliyun-sdk-oss = "com.aliyun.oss:aliyun-sdk-oss:3.10.2"
+antlr-antlr4 = { module = "org.antlr:antlr4", version.ref = "antlr" }
+antlr-runtime = { module = "org.antlr:antlr4-runtime", version.ref = "antlr" }
+arrow-memory-netty = "org.apache.arrow:arrow-memory-netty:11.0.0"
+arrow-vector = "org.apache.arrow:arrow-vector:11.0.0"
+avro-avro = "org.apache.avro:avro:1.11.1"
+awssdk-bom = { module = "software.amazon.awssdk:bom", version = "2.20.18" }
+caffeine = "com.github.ben-manes.caffeine:caffeine:2.9.3"
+calcite-core = "org.apache.calcite:calcite-core:1.10.0"
+calcite-druid = "org.apache.calcite:calcite-druid:1.10.0"
+delta-standalone = { module = "io.delta:delta-standalone_2.12", version.ref = 
"deltaStandalone" }
+errorprone-annotations = "com.google.errorprone:error_prone_annotations:2.3.3"

Review Comment:
   I think it makes sense to extract all versions into the [versions] section, 
or is there a particular reason to not do it?



##########
build.gradle:
##########
@@ -461,46 +455,49 @@ project(':iceberg-aws') {
     api project(':iceberg-api')
     implementation project(':iceberg-common')
     implementation project(':iceberg-core')
-    annotationProcessor "org.immutables:value"
-    compileOnly "org.immutables:value"
-    implementation "com.github.ben-manes.caffeine:caffeine"
+    annotationProcessor libs.immutables.value
+    compileOnly libs.immutables.value
+    implementation libs.caffeine
+    implementation platform(libs.jackson.bom)
     implementation "com.fasterxml.jackson.core:jackson-databind"
     implementation "com.fasterxml.jackson.core:jackson-core"
 
-    compileOnly 'software.amazon.awssdk:url-connection-client'
-    compileOnly 'software.amazon.awssdk:apache-client'
-    compileOnly 'software.amazon.awssdk:auth'
-    compileOnly 'software.amazon.awssdk:s3'
-    compileOnly 'software.amazon.awssdk:kms'
-    compileOnly 'software.amazon.awssdk:glue'
-    compileOnly 'software.amazon.awssdk:sts'
-    compileOnly 'software.amazon.awssdk:dynamodb'
-    compileOnly 'software.amazon.awssdk:lakeformation'
-
-    compileOnly("org.apache.hadoop:hadoop-common") {
+    compileOnly(platform(libs.awssdk.bom))

Review Comment:
   I think we should probably do the same here as I mentioned further above for 
jackson-databind



##########
build.gradle:
##########
@@ -854,28 +858,29 @@ project(':iceberg-nessie') {
     implementation project(':iceberg-common')
     implementation project(':iceberg-core')
     implementation project(path: ':iceberg-bundled-guava', configuration: 
'shadow')
-    implementation "org.projectnessie.nessie:nessie-client"
+    implementation libs.nessie.client
+    implementation platform(libs.jackson.bom)

Review Comment:
   same as above



##########
build.gradle:
##########
@@ -539,13 +536,14 @@ project(':iceberg-delta-lake') {
     implementation project(':iceberg-common')
     implementation project(':iceberg-core')
     implementation project(':iceberg-parquet')
+    implementation platform(libs.jackson.bom)

Review Comment:
   same as above



##########
gradle/libs.versions.toml:
##########
@@ -0,0 +1,153 @@
+# Also see settings.gradle file for dynamic versionCatalogs dependencies
+
+[versions]
+antlr = "4.9.3"
+deltaStandalone = "0.6.0"
+deltaCore = "2.2.0"
+flink115 = { strictly = "[1.15, 1.16[", prefer = "1.15.0"}
+flink116 = { strictly = "[1.16, 1.17[", prefer = "1.16.2"}
+flink117 = { strictly = "[1.17, 1.18[", prefer = "1.17.1"}
+guava = "32.1.1-jre"
+hadoop2 = "2.7.3"
+hive2 = { strictly = "[2, 3[", prefer = "2.3.9"}
+hive3 = "3.1.3"
+jackson211 = "2.11.4"
+jackson212 = "2.12.3"
+jackson213 = "2.13.4"
+jetty = "9.4.43.v20210629"
+junitJupiter = "5.9.2"
+mockito = "4.11.0"
+nessie = "0.63.0"
+orc = "1.9.0"
+parquet = "1.13.1"
+scalaCollectionCompat = "2.6.0"
+sparkHive31 = "3.1.3"
+sparkHive32 = "3.2.2"
+sparkHive33 = "3.3.2"
+sparkHive34 = "3.4.1"
+springBoot = "2.5.4"
+tez010 = "0.10.2"
+tez08 = { strictly = "[0.8, 0.9[", prefer = "0.8.4"}

Review Comment:
   any particular reason for the version range vs just defining the version?



##########
flink/v1.17/build.gradle:
##########
@@ -97,7 +107,7 @@ project(":iceberg-flink:iceberg-flink-${flinkMajorVersion}") 
{
       exclude group: 'com.google.code.findbugs', module: 'jsr305'
     }
 
-    testImplementation("org.apache.hive:hive-metastore") {
+    testImplementation(libs.hive2.metastore) {

Review Comment:
   L126 needs to be changed to `testImplementation libs.awaitility`. Same for 
the other Flink versions



##########
spark/v3.1/build.gradle:
##########
@@ -131,20 +131,21 @@ 
project(":iceberg-spark:iceberg-spark-extensions-3.1_2.12") {
   }
 
   dependencies {
-    compileOnly "org.scala-lang:scala-library"
+    compileOnly "org.scala-lang:scala-library:${scalaVersion}"
     compileOnly project(path: ':iceberg-bundled-guava', configuration: 
'shadow')
     compileOnly project(':iceberg-api')
     compileOnly project(':iceberg-core')
     compileOnly project(':iceberg-data')
     compileOnly project(':iceberg-orc')
     compileOnly project(':iceberg-common')
     compileOnly project(':iceberg-spark:iceberg-spark-3.1_2.12')
-    compileOnly("org.apache.spark:spark-hive_2.12:${sparkVersion}") {
+    
compileOnly("org.apache.spark:spark-hive_${scalaVersion}:${libs.versions.sparkHive31.get()}")
 {
       exclude group: 'org.apache.avro', module: 'avro'
       exclude group: 'org.apache.arrow'
       exclude group: 'org.apache.parquet'
       exclude group: 'org.roaringbitmap'
     }
+    compileOnly platform(libs.jackson211.bom)

Review Comment:
   why do we need to add this dependency now?



##########
spark/v3.2/build.gradle:
##########
@@ -73,25 +69,29 @@ 
project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}") {
       exclude group: 'io.netty', module: 'netty-common'
       exclude group: 'org.roaringbitmap'
     }
+    compileOnly platform(libs.jackson212.bom)

Review Comment:
   is this needed here?



##########
spark/v3.2/build.gradle:
##########
@@ -60,11 +56,11 @@ 
project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}") {
     implementation project(':iceberg-orc')
     implementation project(':iceberg-parquet')
     implementation project(':iceberg-arrow')
-    
implementation("org.scala-lang.modules:scala-collection-compat_${scalaVersion}")
+    implementation 
"org.scala-lang.modules:scala-collection-compat_${scalaVersion}:2.6.0"

Review Comment:
   why do we need the `2.6.0` now?



##########
spark/v3.2/build.gradle:
##########
@@ -60,11 +56,11 @@ 
project(":iceberg-spark:iceberg-spark-${sparkMajorVersion}_${scalaVersion}") {
     implementation project(':iceberg-orc')
     implementation project(':iceberg-parquet')
     implementation project(':iceberg-arrow')
-    
implementation("org.scala-lang.modules:scala-collection-compat_${scalaVersion}")
+    implementation 
"org.scala-lang.modules:scala-collection-compat_${scalaVersion}:2.6.0"
 
-    compileOnly "com.google.errorprone:error_prone_annotations"
-    compileOnly "org.apache.avro:avro"
-    compileOnly("org.apache.spark:spark-hive_${scalaVersion}:${sparkVersion}") 
{
+    compileOnly(libs.errorprone.annotations)

Review Comment:
   nit: parentheses can be removed



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to