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

[WIP][SPARK-29750][BUILD] Avoid dependency from joda-time #26392

Closed
wants to merge 2 commits into from

Conversation

MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Nov 5, 2019

What changes were proposed in this pull request?

Removed direct dependency from joda-time.

Why are the changes needed?

To reduce number of direct Spark dependencies.

Does this PR introduce any user-facing change?

No, only if users depend on joda-time transitively.

How was this patch tested?

By building Spark via ./build/sbt -Phive -Phive-thriftserver package

@SparkQA
Copy link

SparkQA commented Nov 5, 2019

Test build #113242 has finished for PR 26392 at commit cb8c387.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@@ -125,7 +125,6 @@ jetty-6.1.26.jar
jetty-sslengine-6.1.26.jar
jetty-util-6.1.26.jar
jline-2.14.6.jar
joda-time-2.10.5.jar
Copy link
Member

Choose a reason for hiding this comment

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

Instead of this, joda-time-2.9.9.jar seems to occur transitively. Could you check the dependency tree and remove that, too?

+++ b/dev/pr-deps/spark-deps-hadoop-2.7
@@ -125,6 +125,7 @@ jetty-6.1.26.jar
 jetty-sslengine-6.1.26.jar
 jetty-util-6.1.26.jar
 jline-2.14.6.jar
+joda-time-2.9.9.jar
 jodd-core-3.5.2.jar
 jpam-1.1.jar
 json4s-ast_2.12-3.6.6.jar

@@ -139,6 +139,7 @@ jersey-server-2.29.jar
jetty-webapp-9.4.18.v20190429.jar
jetty-xml-9.4.18.v20190429.jar
jline-2.14.6.jar
joda-time-2.8.1.jar
Copy link
Member

Choose a reason for hiding this comment

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

Ur, are you suggesting to have two difference (lower) joda-time version in this commit??? ;)

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Sorry, but I'm -1 to downgrade like this. If this PR is not aiming the removal of joda-time completely, let's not do this.

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 5, 2019

Sorry, but I'm -1 to downgrade like this.

@dongjoon-hyun You are so quick.

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 5, 2019

arrow vector requires joda time in 12.x:

[info] joda-time:joda-time:2.9.9
[info]   +-org.apache.arrow:arrow-vector:0.12.0

https://github.com/apache/arrow/blob/apache-arrow-0.12.0/java/vector/pom.xml#L34-L38
but arrow 0.15.1 does not require it anymore: https://github.com/apache/arrow/blob/apache-arrow-0.15.1/java/vector/pom.xml

@dongjoon-hyun
Copy link
Member

Is that the last one? Or, just one dependency instance?

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 5, 2019

Is that the last one? Or, just one dependency instance?

I bumped arrow version to 0.15.1 locally, so, there is no dependency from joda-time anymore. Not sure that is is safe to migrate to arrow vector 0.15.1

@dongjoon-hyun
Copy link
Member

I pinged you the on-going Arrow upgrade PR. Let's resume this PR after that PR.

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 5, 2019

There is a transitive dependency from joda-time via hive-common for hadoop-3.2:

[info] joda-time:joda-time:2.8.1
[info]   +-org.apache.hive:hive-common:2.3.6
[info]     +-org.apache.spark:spark-hive_2.12:3.0.0-SNAPSHOT [S]

which still exists in hive's master: https://github.com/apache/hive/blob/master/common/pom.xml#L101-L105

@SparkQA
Copy link

SparkQA commented Nov 5, 2019

Test build #113243 has finished for PR 26392 at commit 02e9bee.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 6, 2019

I am closing this because there is no chance of hive-common avoids dependency from joda-time any time soon.

@MaxGekk MaxGekk closed this Nov 6, 2019
@MaxGekk MaxGekk deleted the remove-joda-time branch June 5, 2020 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants