You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "jbonofre (via GitHub)" <gi...@apache.org> on 2025/06/09 00:17:06 UTC

[PR] Fix LICENSE and NOTICE in Spark plugin [polaris]

jbonofre opened a new pull request, #1834:
URL: https://github.com/apache/polaris/pull/1834

   This PR fixes `LICENSE` and `NOTICE` in the Polaris Spark plugin.
   
   @flyrain @gh-yzou I'm observing some "issues" in the Spark plugin. I see some packages are shaded multiple times. For instance Jackson is shaded in Iceberg and we shade again in Polaris Spark plugin.
   Imho, the Polaris Spark plugin needs some cleanup in the bundle artifact.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix LICENSE and NOTICE in Spark plugin [polaris]

Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on code in PR #1834:
URL: https://github.com/apache/polaris/pull/1834#discussion_r2136050744


##########
plugins/spark/v3.5/spark/LICENSE:
##########
@@ -202,124 +202,368 @@
 
 --------------------------------------------------------------------------------
 
-This product includes a gradle wrapper.
-
-* gradlew
-* gradle/wrapper/gradle-wrapper.properties
-
-Copyright: 2010-2019 Gradle Authors.
-Home page: https://github.com/gradle/gradle
-License: https://www.apache.org/licenses/LICENSE-2.0
-
---------------------------------------------------------------------------------
-
 This product includes code from Apache Iceberg.
 
-* spec/iceberg-rest-catalog-open-api.yaml
-* spec/polaris-catalog-apis/oauth-tokens-api.yaml
-* integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java
-* service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
-* service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java
 * plugins/spark/v3.5/spark/src/main/java/org/apache/polaris/spark/PolarisSparkCatalog.java
 * plugins/spark/v3.5/spark/src/test/java/org/apache/polaris/spark/PolarisInMemoryCatalog.java

Review Comment:
   Yes, those two are not copied from iceberg, those are Polaris specific with generic table implementations



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix LICENSE and NOTICE in Spark plugin [polaris]

Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #1834:
URL: https://github.com/apache/polaris/pull/1834#issuecomment-2954471840

   @gh-yzou yeah, we can keep the duplicate dependencies (there are several dependencies). Let me do a quick check (listing the duplicate dependencies). 


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix LICENSE and NOTICE in Spark plugin [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #1834:
URL: https://github.com/apache/polaris/pull/1834#discussion_r2136008430


##########
plugins/spark/v3.5/spark/LICENSE:
##########
@@ -202,124 +202,368 @@
 
 --------------------------------------------------------------------------------
 
-This product includes a gradle wrapper.
-
-* gradlew
-* gradle/wrapper/gradle-wrapper.properties
-
-Copyright: 2010-2019 Gradle Authors.
-Home page: https://github.com/gradle/gradle
-License: https://www.apache.org/licenses/LICENSE-2.0
-
---------------------------------------------------------------------------------
-
 This product includes code from Apache Iceberg.
 
-* spec/iceberg-rest-catalog-open-api.yaml
-* spec/polaris-catalog-apis/oauth-tokens-api.yaml
-* integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationTest.java
-* service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
-* service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java
 * plugins/spark/v3.5/spark/src/main/java/org/apache/polaris/spark/PolarisSparkCatalog.java
 * plugins/spark/v3.5/spark/src/test/java/org/apache/polaris/spark/PolarisInMemoryCatalog.java

Review Comment:
   These two lines are also not needed per offline discussion, as they don't copy any code from Iceberg. Can you confirm, @gh-yzou ? 



-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix LICENSE and NOTICE in Spark plugin [polaris]

Posted by "gh-yzou (via GitHub)" <gi...@apache.org>.
gh-yzou commented on PR #1834:
URL: https://github.com/apache/polaris/pull/1834#issuecomment-2954445538

   @jbonofre Thanks a lot for helping on the license. Regarding to the shaded Jackson library, it was shaded again by the polaris plugin because we are not able to reuse the shaded Jackson library due to inconsistency issue. so we introduces the dependency again, and shaded in the packaged jar, and since we also included the whole iceberg spark client, which also shaded the jackson library, so we end up shaded jackson twice. Can we keep both shade for the current jar?


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix LICENSE and NOTICE in Spark plugin [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #1834:
URL: https://github.com/apache/polaris/pull/1834#issuecomment-2956446675

   Thanks @jbonofre for the fix. Thanks @gh-yzou for the review.


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Fix LICENSE and NOTICE in Spark plugin [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain merged PR #1834:
URL: https://github.com/apache/polaris/pull/1834


-- 
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: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org