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.

Sign up for GitHub

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

Jump to bottom

[HUDI-1540] Fixing commons codec depedency in bundle jars #2562

Merged
merged 1 commit into from
Mar 1, 2021
Merged

[HUDI-1540] Fixing commons codec depedency in bundle jars #2562

merged 1 commit into from
Mar 1, 2021

Conversation

nsivabalan
Copy link
Contributor

What is the purpose of the pull request

commons-codec wasn't pulled in from any of the dep. So had to explicitly add it in our bundles

Brief change log

(for example:)

  • Added commons-codec dependency to spark and utilities bundle

Verify this pull request

manually verified.

tar -tvf packaging/hudi-utilities-bundle/target/hudi-utilities-bundle_2.11-0.8.0-SNAPSHOT.jar | grep commons-codec
drwxrwxr-x  0 0      0           0 Feb 10 07:59 META-INF/maven/commons-codec/
drwxrwxr-x  0 0      0           0 Feb 10 07:59 META-INF/maven/commons-codec/commons-codec/
-rw-rw-r--  0 0      0       10494 Aug  6  2009 META-INF/maven/commons-codec/commons-codec/pom.xml
-rw-rw-r--  0 0      0         114 Aug  6  2009 META-INF/maven/commons-codec/commons-codec/pom.properties
sivabala-C02XG219JGH6:hudi sivabala$ tar -tvf packaging/hudi-spark-bundle/target/hudi-spark-bundle_2.11-0.8.0-SNAPSHOT.jar | grep commons-codec
drwxrwxr-x  0 0      0           0 Feb 10 08:00 META-INF/maven/commons-codec/
drwxrwxr-x  0 0      0           0 Feb 10 08:00 META-INF/maven/commons-codec/commons-codec/
-rw-rw-r--  0 0      0       10494 Aug  6  2009 META-INF/maven/commons-codec/commons-codec/pom.xml
-rw-rw-r--  0 0      0         114 Aug  6  2009 META-INF/maven/commons-codec/commons-codec/pom.properties

Committer checklist

  • Has a corresponding JIRA in PR title & commit

  • Commit message is descriptive of the change

  • CI is green

  • Necessary doc changes done or have another open PR

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

@nsivabalan nsivabalan mentioned this pull request Feb 10, 2021
Closed
5 tasks
@codecov-io
Copy link

codecov-io commented Feb 10, 2021 •
edited

Codecov Report

Merging #2562 (5258866) into master (a2f85d9) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2562      +/-   ##
============================================
- Coverage     50.90%   50.90%   -0.01%     
- Complexity     3167     3168       +1     
============================================
  Files           433      433              
  Lines         19806    19806              
  Branches       2032     2032              
============================================
- Hits          10083    10082       -1     
- Misses         8904     8905       +1     
  Partials        819      819
Flag Coverage Δ Complexity Δ
hudicli 36.90% <ø> (ø) 0.00 <ø> (ø)
hudiclient 100.00% <ø> (ø) 0.00 <ø> (ø)
hudicommon 51.38% <ø> (-0.03%) 0.00 <ø> (ø)
hudiflink 43.21% <ø> (ø) 0.00 <ø> (ø)
hudihadoopmr 33.16% <ø> (ø) 0.00 <ø> (ø)
hudisparkdatasource 69.46% <ø> (ø) 0.00 <ø> (ø)
hudisync 48.61% <ø> (ø) 0.00 <ø> (ø)
huditimelineservice 66.49% <ø> (ø) 0.00 <ø> (ø)
hudiutilities 69.51% <ø> (+0.05%) 0.00 <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
...e/hudi/common/table/log/HoodieLogFormatWriter.java 78.12% <0.00%> (-1.57%) 26.00% <0.00%> (ø%)
...apache/hudi/utilities/deltastreamer/DeltaSync.java 70.86% <0.00%> (+0.35%) 51.00% <0.00%> (+1.00%)

n3nash
n3nash approved these changes Feb 10, 2021
View reviewed changes
Copy link
Contributor

@n3nash n3nash left a comment

Choose a reason for hiding this comment

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

LGTM

@n3nash
Copy link
Contributor

n3nash commented Feb 10, 2021

@nsivabalan can you confirm that the package is relocated and not a hard dependency on the actual package ?

@vinothchandar
Copy link
Member

so the reason, this was left out was that we could use it from spark/hive etc, who should already have this? whats the actual problem we are addressing here , by including these jars?

@vinothchandar
Copy link
Member

+1 on relocation. we need to be really sure about relocating or not.

@nsivabalan nsivabalan added priority:critical production down; pipelines stalled; Need help asap. priority:major degraded perf; unable to move forward; potential bugs and removed priority:major degraded perf; unable to move forward; potential bugs labels Feb 11, 2021
@nsivabalan
Copy link
Contributor Author

mvn dependency:tree -Dincludes=commons-codec:commons-codec

.
.
[INFO] ---------------< org.apache.hudi:hudi-spark-bundle_2.11 >---------------
[INFO] Building hudi-spark-bundle_2.11 0.8.0-SNAPSHOT                   [24/42]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ hudi-spark-bundle_2.11 ---
[INFO] org.apache.hudi:hudi-spark-bundle_2.11:jar:0.8.0-SNAPSHOT
[INFO] \- org.apache.hive:hive-service:jar:2.3.1:provided
[INFO]    \- commons-codec:commons-codec:jar:1.4:compile
.
.
[INFO] ------------------< org.apache.hudi:hudi-spark2_2.11 >------------------
[INFO] Building hudi-spark2_2.11 0.8.0-SNAPSHOT                         [12/42]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ hudi-spark2_2.11 ---
[INFO] org.apache.hudi:hudi-spark2_2.11:jar:0.8.0-SNAPSHOT
[INFO] \- org.apache.hudi:hudi-client-common:jar:0.8.0-SNAPSHOT:compile
[INFO]    \- org.apache.hadoop:hadoop-hdfs:jar:tests:2.7.3:compile
[INFO]       \- commons-codec:commons-codec:jar:1.4:compile
[INFO] 
[INFO] ------------------< org.apache.hudi:hudi-spark3_2.12 >------------------
[INFO] Building hudi-spark3_2.12 0.8.0-SNAPSHOT                         [13/42]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ hudi-spark3_2.12 ---
[INFO] org.apache.hudi:hudi-spark3_2.12:jar:0.8.0-SNAPSHOT
[INFO] \- org.apache.spark:spark-sql_2.12:jar:3.0.0:compile (optional) 
[INFO]    \- org.apache.spark:spark-catalyst_2.12:jar:3.0.0:compile (optional) 
[INFO]       \- commons-codec:commons-codec:jar:1.10:compile
[INFO] 
[INFO] ------------------< org.apache.hudi:hudi-spark_2.11 >-------------------
[INFO] Building hudi-spark_2.11 0.8.0-SNAPSHOT                          [14/42]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ hudi-spark_2.11 ---
[INFO] org.apache.hudi:hudi-spark_2.11:jar:0.8.0-SNAPSHOT
[INFO] \- org.apache.hadoop:hadoop-common:jar:2.7.3:provided
[INFO]    \- commons-codec:commons-codec:jar:1.4:compile
.
.
[INFO] ----------------< org.apache.hudi:hudi-utilities_2.11 >-----------------
[INFO] Building hudi-utilities_2.11 0.8.0-SNAPSHOT                      [15/42]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ hudi-utilities_2.11 ---
[INFO] org.apache.hudi:hudi-utilities_2.11:jar:0.8.0-SNAPSHOT
[INFO] \- org.apache.hadoop:hadoop-hdfs:jar:tests:2.7.3:test
[INFO]    \- commons-codec:commons-codec:jar:1.4:compile
.
[INFO] -------------< org.apache.hudi:hudi-utilities-bundle_2.11 >-------------
[INFO] Building hudi-utilities-bundle_2.11 0.8.0-SNAPSHOT               [16/42]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:3.1.1:tree (default-cli) @ hudi-utilities-bundle_2.11 ---
[INFO] org.apache.hudi:hudi-utilities-bundle_2.11:jar:0.8.0-SNAPSHOT
[INFO] \- org.apache.hive:hive-service:jar:2.3.1:provided
[INFO]    \- commons-codec:commons-codec:jar:1.4:compile
.
.

Full output: https://gist.github.com/nsivabalan/aa5b78c8346045e966b2b916821cbed6

@nsivabalan
Copy link
Contributor Author

and yes, it is relocated as well.

tar -tvf packaging/hudi-utilities-bundle/target/hudi-utilities-bundle_2.11-0.8.0-SNAPSHOT.jar | grep commons.codec | head -3
drwxrwxr-x 0 0 0 0 Feb 13 15:39 org/apache/hudi/org/apache/commons/codec/
drwxrwxr-x 0 0 0 0 Feb 13 15:39 org/apache/hudi/org/apache/commons/codec/binary/
-rw-rw-r-- 0 0 0 11317 Feb 13 15:39 org/apache/hudi/org/apache/commons/codec/binary/Base64.class

@nsivabalan
Copy link
Contributor Author

@vinothchandar @n3nash : gentle ping.

@vinothchandar vinothchandar merged commit 657e73f into apache:master Mar 1, 2021
@vinothchandar
Copy link
Member

cc @yanghua @garyli1019 @leesf to consider this for flink bundle as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@n3nash n3nash n3nash approved these changes

Assignees
No one assigned
Labels
priority:critical production down; pipelines stalled; Need help asap.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nsivabalan @codecov-io @n3nash @vinothchandar