[HUDI-479] Eliminate or Minimize use of Guava if possible #1159

smarthi · 2019-12-30T19:35:52Z

What is the purpose of the pull request

Minimize the use of com.google.guava calls in Hudi

Brief changelog

Minimize the use of Guava Calls

Verify this pull request

(Please pick either of the following options)

This pull request is a trivial rework / code cleanup without any test coverage.

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.

hudi-client/src/main/java/org/apache/hudi/index/bloom/BloomIndexFileInfo.java

vinothchandar

Thanks for the great cleanup @smarthi ! left a few minor comments

hudi-client/src/main/java/org/apache/hudi/index/bloom/BloomIndexFileInfo.java

hudi-client/src/main/java/org/apache/hudi/io/compact/HoodieRealtimeTableCompactor.java

hudi-client/src/main/java/org/apache/hudi/table/RollbackExecutor.java

hudi-common/src/main/java/org/apache/hudi/common/model/HoodieKey.java

hudi-common/src/test/java/org/apache/hudi/common/table/TestHoodieTableMetaClient.java

hudi-integ-test/pom.xml

pom.xml

smarthi · 2020-03-26T00:54:42Z

@smarthi is this ready for final review? Have we eliminated Guava at this point?

@vinothchandar please review

vinothchandar

Left a few comments.. one more crank of the wheel and we should be ready to merge

hudi-cli/src/test/java/org/apache/hudi/cli/common/HoodieTestCommitMetadataGenerator.java

hudi-common/src/main/java/org/apache/hudi/common/util/CleanerUtils.java

hudi-common/src/main/java/org/apache/hudi/common/util/CollectionUtils.java

vinothchandar · 2020-03-26T05:18:14Z

hudi-common/src/main/java/org/apache/hudi/common/util/FileIOUtils.java

did we reuse this code from somewhere?

vinothchandar · 2020-03-26T05:19:17Z

hudi-common/src/main/java/org/apache/hudi/common/util/FileIOUtils.java

just the following in the try block?

if (closeable != null) { closeable.close(); }

hudi-common/src/main/java/org/apache/hudi/common/util/FileIOUtils.java

hudi-common/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java

vinothchandar · 2020-03-26T16:30:47Z

@smarthi The CollectionUtils IMO does not seem to reuse any code from guava actually.. it just wraps Java Streams/Collection apis in most cases.. SO we need not attribute anything in LICENSE/NOTICE.. Please clarify

smarthi · 2020-03-26T16:58:14Z

@smarthi The CollectionUtils IMO does not seem to reuse any code from guava actually.. it just wraps Java Streams/Collection apis in most cases.. SO we need not attribute anything in LICENSE/NOTICE.. Please clarify

that's Correct. No attribution needed in LICENSE or NOTICe whatsoever.

hudi-common/src/main/java/org/apache/hudi/common/util/FileIOUtils.java

vinothchandar

LGTM.. @smarthi just one pending issue in FileIOUtils . Please self merge after resolving

vinothchandar · 2020-03-27T14:41:11Z

[INFO] There is 1 error reported by Checkstyle 8.18 with style/checkstyle.xml ruleset.
593[ERROR] src/main/java/org/apache/hudi/avro/MercifulJsonConverter.java:[33] (imports) AvoidStarImport: Using the '.*' form of import should be avoided - java.util.*.

there are check style failures? to be resolved?

smarthi · 2020-03-27T14:53:36Z

[INFO] There is 1 error reported by Checkstyle 8.18 with style/checkstyle.xml ruleset.
593[ERROR] src/main/java/org/apache/hudi/avro/MercifulJsonConverter.java:[33] (imports) AvoidStarImport: Using the '.*' form of import should be avoided - java.util.*.

there are check style failures? to be resolved?

Resolved

vinothchandar · 2020-03-27T14:58:54Z

If its already used elsewhere, nvm. We can deal with it later

… to prevent OffsetOutOfRange exception (apache#1159) Signed-off-by: Roushan Kumar <[email protected]> Co-authored-by: Tim Brown <[email protected]>

smarthi requested a review from bvaradar December 30, 2019 19:35

smarthi force-pushed the HUDI-479 branch 3 times, most recently from 5d8d12f to ac0e30e Compare December 30, 2019 22:20

cdmikechen reviewed Dec 31, 2019

View reviewed changes

hudi-client/src/main/java/org/apache/hudi/index/bloom/BloomIndexFileInfo.java Outdated Show resolved Hide resolved

smarthi force-pushed the HUDI-479 branch 9 times, most recently from eb926a6 to 07a164f Compare December 31, 2019 09:23

smarthi requested a review from vinothchandar December 31, 2019 09:59

vinothchandar changed the title ~~HUDI-479: Eliminate or Minimize use of Guava if possible~~ [HUDI-479]: Eliminate or Minimize use of Guava if possible Dec 31, 2019

vinothchandar changed the title ~~[HUDI-479]: Eliminate or Minimize use of Guava if possible~~ [HUDI-479] Eliminate or Minimize use of Guava if possible Dec 31, 2019

vinothchandar self-assigned this Dec 31, 2019

smarthi force-pushed the HUDI-479 branch 4 times, most recently from 3dd8971 to 4c1579f Compare December 31, 2019 19:54

vinothchandar reviewed Dec 31, 2019

View reviewed changes

smarthi force-pushed the HUDI-479 branch 3 times, most recently from 438e849 to 9d5843c Compare December 31, 2019 22:09

smarthi changed the title ~~[HUDI-479] Eliminate or Minimize use of Guava if possible~~ [WIP][HUDI-479] Eliminate or Minimize use of Guava if possible Jan 2, 2020

smarthi force-pushed the HUDI-479 branch 3 times, most recently from b409cd8 to e296034 Compare January 2, 2020 08:11

smarthi force-pushed the HUDI-479 branch 4 times, most recently from 0fe7466 to 41059b5 Compare March 26, 2020 00:41

smarthi force-pushed the HUDI-479 branch from 41059b5 to 7d3fb91 Compare March 26, 2020 03:41

vinothchandar reviewed Mar 26, 2020

View reviewed changes

smarthi force-pushed the HUDI-479 branch 2 times, most recently from 3bb4f81 to 00004ce Compare March 27, 2020 02:47

vinothchandar reviewed Mar 27, 2020

View reviewed changes

hudi-common/src/main/java/org/apache/hudi/common/util/FileIOUtils.java Outdated Show resolved Hide resolved

vinothchandar approved these changes Mar 27, 2020

View reviewed changes

smarthi force-pushed the HUDI-479 branch 3 times, most recently from 914c6f5 to 49e5919 Compare March 27, 2020 14:49

smarthi force-pushed the HUDI-479 branch 4 times, most recently from d586cb5 to 676fa0f Compare March 28, 2020 02:34

HUDI-479: Eliminate or Minimize use of Guava if possible

e62f86d

smarthi force-pushed the HUDI-479 branch from 676fa0f to e62f86d Compare March 28, 2020 06:32

smarthi merged commit 8c30013 into apache:master Mar 28, 2020

smarthi deleted the HUDI-479 branch March 28, 2020 07:12

lyogev pushed a commit to YotpoLtd/incubator-hudi that referenced this pull request Mar 30, 2020

HUDI-479: Eliminate or Minimize use of Guava if possible (apache#1159)

011a039

[HUDI-479] Eliminate or Minimize use of Guava if possible #1159

[HUDI-479] Eliminate or Minimize use of Guava if possible #1159

Uh oh!

Conversation

smarthi commented Dec 30, 2019

What is the purpose of the pull request

Brief changelog

Verify this pull request

Committer checklist

Uh oh!

Uh oh!

vinothchandar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smarthi commented Mar 26, 2020

Uh oh!

vinothchandar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vinothchandar Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

vinothchandar Mar 26, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vinothchandar commented Mar 26, 2020

Uh oh!

smarthi commented Mar 26, 2020

Uh oh!

Uh oh!

vinothchandar left a comment

Choose a reason for hiding this comment

Uh oh!

vinothchandar commented Mar 27, 2020

Uh oh!

smarthi commented Mar 27, 2020

Uh oh!

vinothchandar commented Mar 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants