elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
69.58k stars 24.63k forks source link

Make Joda dependency optional #78891

Open OverDrone opened 2 years ago

OverDrone commented 2 years ago

I'm trying to use elastic search in a spring boot. I've included code and configuration similar to an example, but that's not important. At some point during initialization XContentBuilder static code starts which searches for all implementations of XContentBuilderExtension: https://github.com/elastic/elasticsearch/blob/20c9f756d28d116f0667e19a1b531aa80c55bde7/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentBuilder.java#L133 Which in turn tries to instantiate class XContentElasticsearchExtension Which fails with subj because it's referencing missing dependency joda-time: https://github.com/elastic/elasticsearch/blob/20c9f756d28d116f0667e19a1b531aa80c55bde7/server/src/main/java/org/elasticsearch/common/xcontent/XContentElasticsearchExtension.java#L22 Yes, I've noticed that spring-data-elasticsearch declares dependency on joda-time along with bzillion other dependencies I'm not using (and manually excluded). Why I need to include yet another jar to my already huge build only because some serialization library requires this dependency because I MIGHT be using it (which I'm not). You don't use jackson for serialization anymore, you had your reasons. That's ok (although I had to study it and migrate my code), but your new serialization library lacks flexibility jackson provided. Ok, you want to add support for joda-time, make it optional or at least automatically detect whether joda-time is on classpath and only then use it instead of just failing with ClassNotFoundException. I would understand if joda-time would be industry standard like logback, but this library was already ported and included in java (java.time) and has alternative at least (is deprecated at most).

elasticmachine commented 2 years ago

Pinging @elastic/es-delivery (Team:Delivery)

cbuescher commented 2 years ago

For what its worth, we've already moved away from Joda towards Java time in Version 7.0, but need to keep the backward compatibility for Joda around for a while to not break peoples use of e.g. Joda specific date patterns. See https://www.elastic.co/guide/en/elasticsearch/reference/7.x/breaking-changes-7.0.html#breaking_70_java_time_changes for the breaking change in 7.0.

From your question I understand that you are looking for more flexibility to exclude Joda if you are sure you don't use it. Would you mind letting us know the version of Elasticsearch you are using? I updated the issue title and changed this to be an enhancement request because I don't think this is a bug.

OverDrone commented 2 years ago

Sure thing, according to gradlew dependency output I use org.springframework.data:spring-data-elasticsearch:4.2.5 -> org.elasticsearch.client:elasticsearch-rest-high-level-client:7.12.1 -> org.elasticsearch:elasticsearch:7.12.1 Which as I understand is the latest version as for today, at least that was my intention to update to latest versions. I don't mind you classifying and editing this issue as you see fit as long as the main idea is intact. I hope this issue gets enough priority and team capacity to be implemented in some foreseeable future.

mark-vieira commented 2 years ago

I'm not sure there's anything to be done here. Joda time is a required dependency, for reasons @cbuescher mentions. Making this "optional" would require ripping out any static references to these classes and replacing them with reflection so that it can be a purely runtime requirement. That's a pretty big undertaking just to shrink the classpath a bit.

Is there an issue with including the joda dependency or is your motivation just to try and keep your runtime classpath as small as possible? Is it otherwise clashing with some other dependency?

mark-vieira commented 2 years ago

It may also be worth noting that going forward we are deprecating the existing HLRC and folks should move to using https://github.com/elastic/elasticsearch-java. This client has no dependency on Elasticsearch itself, and therefor transitively joda-time, so this problem goes away. I believe the intention is for Spring Data to move to using this client in future versions.