You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/10/09 07:16:08 UTC
[GitHub] [hudi] yanghua opened a new pull request #2156: [MINOR] Fix wrong javadoc and refactor some naming issues
yanghua opened a new pull request #2156:
URL: https://github.com/apache/hudi/pull/2156
## What is the purpose of the pull request
*This pull request fixes wrong javadoc and refactor some naming issues*
## Brief change log
- *Fix wrong javadoc and refactor some naming issues (***rdd*** relevant)*
## Verify this pull request
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #2156: [MINOR] Fix wrong javadoc and refactor some naming issues
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2156:
URL: https://github.com/apache/hudi/pull/2156#discussion_r502326023
##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/AbstractWriteHelper.java
##########
@@ -79,7 +79,7 @@ public I combineOnCondition(
*
* @param records hoodieRecords to deduplicate
* @param parallelism parallelism or partitions to be used while reducing/deduplicating
- * @return RDD of HoodieRecord already be deduplicated
+ * @return Collection of HoodieRecord already be deduplicated
*/
public I deduplicateRecords(
I records, HoodieTable<T, I, K, O> table, int parallelism) {
Review comment:
Agree to remove the legacy comment for `HoodieIndexUtils `. IMO, we can remain the comment in `HoodieWriteConfig` as a description of that config option.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #2156: [MINOR] Fix wrong javadoc and refactor some naming issues
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2156:
URL: https://github.com/apache/hudi/pull/2156#discussion_r502326023
##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/AbstractWriteHelper.java
##########
@@ -79,7 +79,7 @@ public I combineOnCondition(
*
* @param records hoodieRecords to deduplicate
* @param parallelism parallelism or partitions to be used while reducing/deduplicating
- * @return RDD of HoodieRecord already be deduplicated
+ * @return Collection of HoodieRecord already be deduplicated
*/
public I deduplicateRecords(
I records, HoodieTable<T, I, K, O> table, int parallelism) {
Review comment:
Agree to remove the legacy comment for `HoodieIndexUtils `. IMO, we can remain the comment in `HoodieWriteConfig` as a description of that config option.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] wangxianghu commented on a change in pull request #2156: [MINOR] Fix wrong javadoc and refactor some naming issues
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2156:
URL: https://github.com/apache/hudi/pull/2156#discussion_r502318018
##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/AbstractWriteHelper.java
##########
@@ -79,7 +79,7 @@ public I combineOnCondition(
*
* @param records hoodieRecords to deduplicate
* @param parallelism parallelism or partitions to be used while reducing/deduplicating
- * @return RDD of HoodieRecord already be deduplicated
+ * @return Collection of HoodieRecord already be deduplicated
*/
public I deduplicateRecords(
I records, HoodieTable<T, I, K, O> table, int parallelism) {
Review comment:
Hi, @yanghua Thanks for raising this pr.
LGTM overall. and I noticed that there are still two lines contains `RDD` in `hudi-client-common` module. maybe we can improve them together.
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/HoodieIndexUtils.java:76
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:121
##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/AbstractWriteHelper.java
##########
@@ -79,7 +79,7 @@ public I combineOnCondition(
*
* @param records hoodieRecords to deduplicate
* @param parallelism parallelism or partitions to be used while reducing/deduplicating
- * @return RDD of HoodieRecord already be deduplicated
+ * @return Collection of HoodieRecord already be deduplicated
*/
public I deduplicateRecords(
I records, HoodieTable<T, I, K, O> table, int parallelism) {
Review comment:
Hi, @yanghua Thanks for raising this pr. LGTM overall.
I noticed that there are still two lines contains `RDD` in `hudi-client-common` module. maybe we can improve them together.
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/HoodieIndexUtils.java:76
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:121
##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/AbstractWriteHelper.java
##########
@@ -79,7 +79,7 @@ public I combineOnCondition(
*
* @param records hoodieRecords to deduplicate
* @param parallelism parallelism or partitions to be used while reducing/deduplicating
- * @return RDD of HoodieRecord already be deduplicated
+ * @return Collection of HoodieRecord already be deduplicated
*/
public I deduplicateRecords(
I records, HoodieTable<T, I, K, O> table, int parallelism) {
Review comment:
> Agree to remove the legacy comment for `HoodieIndexUtils `. IMO, we can remain the comment in `HoodieWriteConfig` as a description of that config option.
Yes, thanks for addressing my concern.
LGTM now
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] wangxianghu commented on a change in pull request #2156: [MINOR] Fix wrong javadoc and refactor some naming issues
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2156:
URL: https://github.com/apache/hudi/pull/2156#discussion_r502330221
##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/AbstractWriteHelper.java
##########
@@ -79,7 +79,7 @@ public I combineOnCondition(
*
* @param records hoodieRecords to deduplicate
* @param parallelism parallelism or partitions to be used while reducing/deduplicating
- * @return RDD of HoodieRecord already be deduplicated
+ * @return Collection of HoodieRecord already be deduplicated
*/
public I deduplicateRecords(
I records, HoodieTable<T, I, K, O> table, int parallelism) {
Review comment:
> Agree to remove the legacy comment for `HoodieIndexUtils `. IMO, we can remain the comment in `HoodieWriteConfig` as a description of that config option.
Yes, thanks for addressing my concern.
LGTM now
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] wangxianghu commented on a change in pull request #2156: [MINOR] Fix wrong javadoc and refactor some naming issues
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2156:
URL: https://github.com/apache/hudi/pull/2156#discussion_r502318018
##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/AbstractWriteHelper.java
##########
@@ -79,7 +79,7 @@ public I combineOnCondition(
*
* @param records hoodieRecords to deduplicate
* @param parallelism parallelism or partitions to be used while reducing/deduplicating
- * @return RDD of HoodieRecord already be deduplicated
+ * @return Collection of HoodieRecord already be deduplicated
*/
public I deduplicateRecords(
I records, HoodieTable<T, I, K, O> table, int parallelism) {
Review comment:
Hi, @yanghua Thanks for raising this pr. LGTM overall.
I noticed that there are still two lines contains `RDD` in `hudi-client-common` module. maybe we can improve them together.
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/HoodieIndexUtils.java:76
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:121
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] wangxianghu commented on a change in pull request #2156: [MINOR] Fix wrong javadoc and refactor some naming issues
Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #2156:
URL: https://github.com/apache/hudi/pull/2156#discussion_r502318018
##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/AbstractWriteHelper.java
##########
@@ -79,7 +79,7 @@ public I combineOnCondition(
*
* @param records hoodieRecords to deduplicate
* @param parallelism parallelism or partitions to be used while reducing/deduplicating
- * @return RDD of HoodieRecord already be deduplicated
+ * @return Collection of HoodieRecord already be deduplicated
*/
public I deduplicateRecords(
I records, HoodieTable<T, I, K, O> table, int parallelism) {
Review comment:
Hi, @yanghua Thanks for raising this pr.
LGTM overall. and I noticed that there are still two lines contains `RDD` in `hudi-client-common` module. maybe we can improve them together.
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/HoodieIndexUtils.java:76
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:121
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on pull request #2156: [MINOR] Fix wrong javadoc and refactor some naming issues
Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2156:
URL: https://github.com/apache/hudi/pull/2156#issuecomment-706016369
@wangxianghu Can you help to review this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] vinothchandar merged pull request #2156: [MINOR] Fix wrong javadoc and refactor some naming issues
Posted by GitBox <gi...@apache.org>.
vinothchandar merged pull request #2156:
URL: https://github.com/apache/hudi/pull/2156
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on pull request #2156: [MINOR] Fix wrong javadoc and refactor some naming issues
Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2156:
URL: https://github.com/apache/hudi/pull/2156#issuecomment-706016369
@wangxianghu Can you help to review this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org