Removing dependency on apache-commons lang 3, adding necessary classes as needed #444

n3nash · 2018-09-04T09:06:46Z

Added 4 new types of classes to help remove this dependency :

Pair and ImmutablePair
Triple and ImmutableTriple
SerializationUtils
StringUtils

In the past, we have faced many issues such as ClassNotFoundException or NoSuchMethod exception due to conflicting classes from lang3 versions, also the SerializationUtils from lang3 is buggy; it has some issues with instantiating the correct class loaders. Hence, we are removing lang3 dependency to alleviate such issues in the future.

n3nash · 2018-09-04T09:08:33Z

@vinothchandar @bvaradar Please take a pass at this

vinothchandar · 2018-09-05T17:54:04Z

hoodie-common/src/main/java/com/uber/hoodie/common/util/collection/SerializationUtils.java

+ * (NOTE: Adapted from Apache commons-lang3)
+ * This class defines API's to serde an object.
+ */
+public class SerializationUtils {


This should nt be in collections.. just common.util.

Agreed, done

vinothchandar · 2018-09-05T17:54:46Z

hoodie-common/src/main/java/com/uber/hoodie/common/util/collection/StringUtils.java

+/**
+ * Simple utility for operations on strings
+ */
+public class StringUtils {


same here.. should nt be in collection?

Agreed, done

vinothchandar · 2018-09-05T17:55:29Z

...on/src/main/java/com/uber/hoodie/common/util/collection/converter/HoodieRecordConverter.java

@@ -103,7 +103,7 @@ public HoodieRecord getData(byte[] bytes) {
      hoodieRecord.setNewLocation(newLocation);
      return hoodieRecord;
    } catch (IOException io) {
-      throw new HoodieNotSerializableException("Cannot de-serialize value from bytes", io);
+      throw new HoodieSerializableException("Cannot de-serialize value from bytes", io);


Rename: HoodieSerializationException ?

vinothchandar · 2018-09-05T17:56:17Z

hoodie-common/src/main/java/com/uber/hoodie/common/util/collection/tuple/ImmutablePair.java

+ *  limitations under the License.
+ */
+
+package com.uber.hoodie.common.util.collection.tuple;


can we just keep package as common.util.collection. having pair under tuple package seems kind of odd to me

I just borrowed it from lang3, we can keep it under collection, done

vinothchandar · 2018-09-05T17:57:05Z

hoodie-common/src/main/java/com/uber/hoodie/common/util/collection/tuple/Pair.java

+ *  limitations under the License.
+ */
+
+package com.uber.hoodie.common.util.collection.tuple;


same here.. just common.util.collection?

I just borrowed it from lang3, we can keep it under collection, done

vinothchandar · 2018-09-05T17:57:14Z

hoodie-common/src/main/java/com/uber/hoodie/common/util/collection/tuple/ImmutableTriple.java

+ *  limitations under the License.
+ */
+
+package com.uber.hoodie.common.util.collection.tuple;


same here.. just common.util.collection?

I just borrowed it from lang3, we can keep it under collection, done

vinothchandar · 2018-09-05T17:57:29Z

hoodie-common/src/main/java/com/uber/hoodie/common/util/collection/tuple/Triple.java

+ *  limitations under the License.
+ */
+
+package com.uber.hoodie.common.util.collection.tuple;


same here.. just common.util.collection?

I just borrowed it from lang3, we can keep it under collection, done

…s as needed

n3nash · 2018-09-05T18:49:24Z

@vinothchandar updated the diff

bvaradar · 2018-09-05T19:12:54Z

hoodie-common/pom.xml

@@ -110,10 +110,6 @@
      <artifactId>hadoop-common</artifactId>
      <classifier>tests</classifier>
    </dependency>
-    <dependency>


Removing this dependency can sometimes still result in NoSuchMethod or other runtime errors elsewhere because of different versions of commons-lang3 getting pulled due to multiple "transitive" dependencies.

To avoid potential landmines, Can you check using mvn dependency:tree with this code-change to see if different versions of commons-lang3 is appearing in multiple different paths. You would need to use both mvn dependency:tree and parent dependency's pom files to figure out.

Good point, here is the breakdown of where this library is brought in :

hoodie-cli

[INFO] +- org.springframework.shell:spring-shell:jar:1.2.0.RELEASE:compile
[INFO] | +- com.google.guava:guava:jar:15.0:compile
[INFO] | +- jline:jline:jar:2.12:compile
[INFO] | +- org.springframework:spring-context-support:jar:4.2.4.RELEASE:compile
[INFO] | | +- org.springframework:spring-beans:jar:4.2.4.RELEASE:compile
[INFO] | | - org.springframework:spring-context:jar:4.2.4.RELEASE:compile
[INFO] | | +- org.springframework:spring-aop:jar:4.2.4.RELEASE:compile
[INFO] | | | - aopalliance:aopalliance:jar:1.0:compile
[INFO] | | - org.springframework:spring-expression:jar:4.2.4.RELEASE:compile
[INFO] | +- commons-io:commons-io:jar:2.4:compile
[INFO] | - org.springframework:spring-core:jar:4.2.4.RELEASE:compile
[INFO] | - commons-logging:commons-logging:jar:1.2:compile
[INFO] +- de.vandermeer:asciitable:jar:0.2.5:compile
[INFO] | +- de.vandermeer:asciilist:jar:0.0.3:compile
[INFO] | - org.apache.commons:commons-lang3:jar:3.4:compile

hoodie-client

[INFO] +- org.apache.spark:spark-core_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.avro:avro-mapred:jar:hadoop2:1.7.7:provided
[INFO] | +- com.twitter:chill_2.11:jar:0.8.0:provided
[INFO] | | - com.esotericsoftware:kryo-shaded:jar:3.0.3:provided
[INFO] | | +- com.esotericsoftware:minlog:jar:1.3.0:provided
[INFO] | | - org.objenesis:objenesis:jar:2.1:provided
[INFO] | +- com.twitter:chill-java:jar:0.8.0:provided
[INFO] | +- org.apache.xbean:xbean-asm5-shaded:jar:4.4:provided
[INFO] | +- org.apache.spark:spark-launcher_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.spark:spark-network-common_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.spark:spark-network-shuffle_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.spark:spark-unsafe_2.11:jar:2.1.0:provided
[INFO] | +- javax.servlet:javax.servlet-api:jar:3.1.0:provided
[INFO] | +- org.apache.commons:commons-lang3:jar:3.5:provided

hoodie-spark

[INFO] +- org.apache.spark:spark-core_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.avro:avro-mapred:jar:hadoop2:1.7.7:provided
[INFO] | | +- org.apache.avro:avro-ipc:jar:1.7.7:compile
[INFO] | | - org.apache.avro:avro-ipc:jar:tests:1.7.7:compile
[INFO] | +- com.twitter:chill_2.11:jar:0.8.0:provided
[INFO] | | - com.esotericsoftware:kryo-shaded:jar:3.0.3:provided
[INFO] | | +- com.esotericsoftware:minlog:jar:1.3.0:provided
[INFO] | | - org.objenesis:objenesis:jar:2.1:provided
[INFO] | +- com.twitter:chill-java:jar:0.8.0:provided
[INFO] | +- org.apache.xbean:xbean-asm5-shaded:jar:4.4:provided
[INFO] | +- org.apache.spark:spark-launcher_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.spark:spark-network-common_2.11:jar:2.1.0:provided
[INFO] | | - org.fusesource.leveldbjni:leveldbjni-all:jar:1.8:compile
[INFO] | +- org.apache.spark:spark-network-shuffle_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.spark:spark-unsafe_2.11:jar:2.1.0:provided
[INFO] | +- net.java.dev.jets3t:jets3t:jar:0.7.1:compile
[INFO] | +- org.apache.curator:curator-recipes:jar:2.4.0:compile
[INFO] | | - org.apache.curator:curator-framework:jar:2.4.0:compile
[INFO] | +- javax.servlet:javax.servlet-api:jar:3.1.0:provided
[INFO] | +- org.apache.commons:commons-lang3:jar:3.5:compile

hoodie-utilities

[INFO] +- org.apache.spark:spark-core_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.avro:avro-mapred:jar:hadoop2:1.7.7:provided
[INFO] | +- com.twitter:chill_2.11:jar:0.8.0:provided
[INFO] | | - com.esotericsoftware:kryo-shaded:jar:3.0.3:provided
[INFO] | | +- com.esotericsoftware:minlog:jar:1.3.0:provided
[INFO] | | - org.objenesis:objenesis:jar:2.1:provided
[INFO] | +- com.twitter:chill-java:jar:0.8.0:provided
[INFO] | +- org.apache.xbean:xbean-asm5-shaded:jar:4.4:provided
[INFO] | +- org.apache.spark:spark-launcher_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.spark:spark-network-common_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.spark:spark-network-shuffle_2.11:jar:2.1.0:provided
[INFO] | +- org.apache.spark:spark-unsafe_2.11:jar:2.1.0:provided
[INFO] | +- javax.servlet:javax.servlet-api:jar:3.1.0:provided
[INFO] | +- org.apache.commons:commons-lang3:jar:3.5:compile

Seems like spark-core brings it, but it's provided.

I think lang3 dependency in hoodie-cli is coming from hoodie-utilities which will be shaded (with Vinoth's deltastreamer changes). Rest of the dependencies are only through spark-core which is provided as you mentioned. So, we should be good here.

deltastreamer changes are nt checked in.. I plan to redo the patch based on these classes anyway.

… schema (apache#444)

n3nash requested review from bvaradar and vinothchandar September 4, 2018 09:08

vinothchandar reviewed Sep 5, 2018

View reviewed changes

Removing dependency on apache-commons lang 3, adding necessary classe…

7a56d29

…s as needed

n3nash force-pushed the fix_apache_commons_usage branch from 687bff5 to 7a56d29 Compare September 5, 2018 18:49

bvaradar reviewed Sep 5, 2018

View reviewed changes

vinothchandar merged commit 324de29 into apache:master Sep 6, 2018

n3nash mentioned this pull request Sep 7, 2018

Remove dependency on apache common-lang3 #442

Closed

vinishjail97 pushed a commit to vinishjail97/hudi that referenced this pull request Dec 15, 2023

[ENG-4147] Fix json schema conversion when array item type is a union…

ee3edf2

… schema (apache#444)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removing dependency on apache-commons lang 3, adding necessary classes as needed #444

Removing dependency on apache-commons lang 3, adding necessary classes as needed #444

n3nash commented Sep 4, 2018 •

edited

Loading

n3nash commented Sep 4, 2018

vinothchandar Sep 5, 2018

n3nash Sep 5, 2018

vinothchandar Sep 5, 2018

n3nash Sep 5, 2018

vinothchandar Sep 5, 2018

n3nash Sep 5, 2018

vinothchandar Sep 5, 2018

n3nash Sep 5, 2018

vinothchandar Sep 5, 2018

n3nash Sep 5, 2018

vinothchandar Sep 5, 2018

n3nash Sep 5, 2018

vinothchandar Sep 5, 2018

n3nash Sep 5, 2018

n3nash commented Sep 5, 2018

bvaradar Sep 5, 2018

n3nash Sep 5, 2018

bvaradar Sep 5, 2018

n3nash Sep 5, 2018

vinothchandar Sep 6, 2018

Removing dependency on apache-commons lang 3, adding necessary classes as needed #444

Removing dependency on apache-commons lang 3, adding necessary classes as needed #444

Conversation

n3nash commented Sep 4, 2018 • edited Loading

n3nash commented Sep 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n3nash commented Sep 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

n3nash commented Sep 4, 2018 •

edited

Loading