Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sedona-spark-shaded package should exclude some repetitive dependencies. #1584

Open
freamdx opened this issue Sep 12, 2024 · 5 comments
Open

Comments

@freamdx
Copy link

freamdx commented Sep 12, 2024

pom.xml should exclude any dependencies that exist in spark jars, eg:
edu.ucar:cdm-core exclude guava/httpclient/protobuf-java
... ... <dependency> <groupId>edu.ucar</groupId> <artifactId>cdm-core</artifactId> <exclusions> <exclusion> <groupId>com.google.guava</groupId> <artifactId>guava</artifactId> </exclusion> <exclusion> <groupId>org.apache.httpcomponents</groupId> <artifactId>httpclient</artifactId> </exclusion> <exclusion> <groupId>com.google.protobuf</groupId> <artifactId>protobuf-java</artifactId> </exclusion> </exclusions> </dependency> ... ... <artifactSet> <excludes> <exclude>org.scala-lang:scala-library</exclude> <exclude>org.apache.commons:commons-*</exclude> <exclude>commons-pool:commons-pool</exclude> <exclude>commons-lang:commons-lang</exclude> <exclude>commons-io:commons-io</exclude> <exclude>commons-logging:commons-logging</exclude> </excludes> </artifactSet> ... ...

@jiayuasu
Copy link
Member

@freamdx would you mind create a PR to fix this yourself?

@Kontinuation
Copy link
Member

Kontinuation commented Sep 13, 2024

A few notes:

  • cdm-core has to be bundled in the shaded package, we include it in the shaded pom.xml to make it compile rather than provided.
  • guava is better shaded than excluded, since we don't know whether the guava jar shipped with spark is compatible with sedona or not, and shading guava is a common practice.
  • other package exclusion seems to be OK. Apache commons has good backward compatibility and the versions shipped with spark is later than what we bundled.

@zwu-net
Copy link
Contributor

zwu-net commented Sep 24, 2024

I agree with @Kontinuation. Guava is pretty tricky, as I learned it when upgrading Spark.

I'm having trouble understanding why this Google's library has such poor compatibility issues. It seems unexpected from a leading tech company

I'm willing to step in if @jiayuasu approves, given @freamdx's lack of response. However, I some worried proceeding without rigorous testing to mitigate potential risks.

@jiayuasu
Copy link
Member

@zwu-net please feel free to create a PR to fix this

@zwu-net
Copy link
Contributor

zwu-net commented Sep 25, 2024

@jiayuasu (I would like also to bring @james-willis here since I got know him due to DBScan discussion. If you are busy on other tasks, you don't need to participate) @Kontinuation @freamdx

Research Findings and Proposal

After researching the feasibility of modifying Sedona's pom.xml (https://github.com/apache/sedona/blob/master/spark-shaded/pom.xml), I concluded that manual management of library inclusions/exclusions across various Spark and Sedona versions would be overly complex and labor-intensive.

Proposed Solution

To address this challenge, I suggest creating a custom tool to manage pom.xml generation. This approach is inspired by my previous work on a custom transformer of SQLs from Oracle to Snowflake(https://www.linkedin.com/pulse/revolutionizing-data-analysis-custom-transformer-seamless-paul-wu-4euxc/?trackingId=1AyXqiJh1sei4yybC%2FZW9g%3D%3D).

Tool Requirements

The tool should:

  1. Fetch Spark releases (ideally from a prefetched repository to minimize build time and network stability issues)
  2. Generate pom.xml files
  3. Be implemented in Java or Python (with careful dependency management if using Python)

Implementation Considerations

Given the scope of this task, it may take several months for part-time contributors like myself to implement and test. Before proceeding, I'd appreciate feedback on:

  1. The viability of this approach
  2. Potential alternative solutions

Please share your thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants