adutra commented on code in PR #1857:
URL: https://github.com/apache/polaris/pull/1857#discussion_r2144492103


##########
site/content/in-dev/unreleased/polaris-spark-client.md:
##########
@@ -128,3 +128,21 @@ The Polaris Spark client has the following functionality 
limitations:
 3) Rename a Delta table is not supported.
 4) ALTER TABLE ... SET LOCATION is not supported for DELTA table.
 5) For other non-Iceberg tables like csv, it is not supported.
+
+## Iceberg Spark Client compatibility with Polaris Spark Client
+The Polaris Spark client today is not designed to be used with Iceberg Spark 
client together. In other words,
+there is no guarantee provided if both 
`org.apache.iceberg:iceberg-spark-runtime-3.5_2.12:<iceberg_version>` and 
+`org.apache.polaris:polaris-spark-3.5_2.12:<polaris_version>` are provided for 
the Spark `package` configuration.
+
+In order to help the usage of IcebergExtension use the same client, Polaris 
Spark client also ships a version of

Review Comment:
   This needs some rephrasing: "In order to help the usage of IcebergExtension 
use the same client..."



##########
site/content/in-dev/unreleased/polaris-spark-client.md:
##########
@@ -128,3 +128,21 @@ The Polaris Spark client has the following functionality 
limitations:
 3) Rename a Delta table is not supported.
 4) ALTER TABLE ... SET LOCATION is not supported for DELTA table.
 5) For other non-Iceberg tables like csv, it is not supported.
+
+## Iceberg Spark Client compatibility with Polaris Spark Client
+The Polaris Spark client today is not designed to be used with Iceberg Spark 
client together. In other words,
+there is no guarantee provided if both 
`org.apache.iceberg:iceberg-spark-runtime-3.5_2.12:<iceberg_version>` and 
+`org.apache.polaris:polaris-spark-3.5_2.12:<polaris_version>` are provided for 
the Spark `package` configuration.
+
+In order to help the usage of IcebergExtension use the same client, Polaris 
Spark client also ships a version of
+Iceberg Spark client with it, the version information is described in the 
following table:
+
+| Spark Client Version | Iceberg Spark Client Version |
+|----------------------|------------------------------|
+| 1.0.0                | 1.9.0                        |
+
+## Trouble Shooting
+1. When starting spark, sometimes it complains it failed to download a 
package, for example, 

Review Comment:
   ```suggestion
   ## Troubleshooting
   1. When starting Spark, sometimes it complains it failed to download a 
package, for example, 
   ```



##########
site/content/in-dev/unreleased/polaris-spark-client.md:
##########
@@ -128,3 +128,21 @@ The Polaris Spark client has the following functionality 
limitations:
 3) Rename a Delta table is not supported.
 4) ALTER TABLE ... SET LOCATION is not supported for DELTA table.
 5) For other non-Iceberg tables like csv, it is not supported.
+
+## Iceberg Spark Client compatibility with Polaris Spark Client
+The Polaris Spark client today is not designed to be used with Iceberg Spark 
client together. In other words,

Review Comment:
   Is this design decision definitive?
   
   I remember I asked @gh-yzou a while ago:
   
   https://github.com/apache/polaris/pull/1303#discussion_r2028560451
   
   Including the entire Iceberg Spark runtime jar in this artifact has a few 
drawbacks such as: increased jar size (~60Mb), increased complexity for LICENSE 
and NOTICE files.
   
   Just compiling against the Spark runtime jar without including it would imo 
solve these problems.



##########
plugins/spark/v3.5/spark/build.gradle.kts:
##########
@@ -173,8 +176,19 @@ tasks.register<ShadowJar>("createPolarisSparkJar") {
     exclude(dependency("org.apache.avro:avro*.*"))
   }
 
-  relocate("com.fasterxml", "org.apache.polaris.shaded.com.fasterxml.jackson")
+  relocate("com.fasterxml", "org.apache.polaris.shaded.com.fasterxml")
   relocate("org.apache.avro", "org.apache.polaris.shaded.org.apache.avro")
 }
 
-tasks.withType(Jar::class).named("sourcesJar") { 
dependsOn("createPolarisSparkJar") }
+// ensure the shadowJar job is run for both `assemble` and `build` task
+tasks.named("assemble") { dependsOn("shadowJar") }
+
+tasks.named("build") { dependsOn("shadowJar") }
+
+tasks.named<Jar>("jar") {
+  // retain the default jar job, and add a classifier to avoid conflict
+  // with the createPolarisSparkJar. This jar is needed by the task "test",

Review Comment:
   ```suggestion
     // with the shadowJar task. This jar is needed by the task "test",
   ```



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

Reply via email to