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/08/02 03:30:17 UTC

[GitHub] [hudi] Mathieu1124 opened a new pull request #1900: [HUDI-531]Add java doc for hudi test suite general classes

Mathieu1124 opened a new pull request #1900:
URL: https://github.com/apache/hudi/pull/1900


   ## *Tips*
   - *Thank you very much for contributing to Apache Hudi.*
   - *Please review https://hudi.apache.org/contributing.html before opening a pull request.*
   
   ## What is the purpose of the pull request
   
   *Add java doc for hudi test suite general classes*
   
   ## Brief change log
   
   *Add java doc for hudi test suite general classes*
   
   ## 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 #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#discussion_r464144947



##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/scheduler/DagScheduler.java
##########
@@ -48,6 +51,11 @@ public DagScheduler(WorkflowDag workflowDag, HoodieTestSuiteWriter hoodieTestSui
     this.executionContext = new ExecutionContext(null, hoodieTestSuiteWriter, deltaGenerator);
   }
 
+  /**
+   * Method to start execute workflowDags.

Review comment:
       `workflowDags ` -> `workflow DAGs `

##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/BulkInsertNode.java
##########
@@ -24,6 +24,9 @@
 import org.apache.hudi.integ.testsuite.configuration.DeltaConfig.Config;
 import org.apache.spark.api.java.JavaRDD;
 
+/**
+ * Represents a bulkInsert Node in the DAG of operations for a workflow.

Review comment:
       `bulkInsert Node` -> `bulk insert node`?

##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/scheduler/DagScheduler.java
##########
@@ -37,6 +37,9 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+/**
+ * The DagScheduler schedules the workflowDag.

Review comment:
       `DagScheduler ` -> `DAG scheduler`

##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/HiveQueryNode.java
##########
@@ -30,6 +30,9 @@
 import org.apache.hudi.integ.testsuite.dag.ExecutionContext;
 import org.apache.hudi.integ.testsuite.helpers.HiveServiceProvider;
 
+/**
+ * Represents a hive query Node in the DAG of operations for a workflow.

Review comment:
       ditto

##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/CompactNode.java
##########
@@ -26,6 +26,9 @@
 import org.apache.hudi.integ.testsuite.dag.ExecutionContext;
 import org.apache.spark.api.java.JavaRDD;
 
+/**
+ * Represents a compact Node in the DAG of operations for a workflow.

Review comment:
       ditto

##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/CleanNode.java
##########
@@ -20,6 +20,9 @@
 
 import org.apache.hudi.integ.testsuite.dag.ExecutionContext;
 
+/**
+ * Represents a clean Node in the DAG of operations for a workflow.

Review comment:
       `Node` -> `node`?




----------------------------------------------------------------
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 pull request #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
wangxianghu commented on pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#issuecomment-671971132


   @yanghua  this pr is ready for review 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] pratyakshsharma commented on a change in pull request #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#discussion_r466834278



##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/DagNode.java
##########
@@ -76,6 +76,12 @@ public void setParentNodes(List<DagNode<O>> parentNodes) {
     this.parentNodes = parentNodes;
   }
 
+  /**
+   * Execute the {@link DagNode}.
+   *
+   * @param context The context needed for an execute of a node.

Review comment:
       execute -> execution




----------------------------------------------------------------
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 merged pull request #1900: [HUDI-531] Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
yanghua merged pull request #1900:
URL: https://github.com/apache/hudi/pull/1900


   


----------------------------------------------------------------
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] pratyakshsharma commented on a change in pull request #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#discussion_r466834370



##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/nodes/DagNode.java
##########
@@ -76,6 +76,12 @@ public void setParentNodes(List<DagNode<O>> parentNodes) {
     this.parentNodes = parentNodes;
   }
 
+  /**
+   * Execute the {@link DagNode}.
+   *
+   * @param context The context needed for an execute of a node.
+   * @throws Exception Thrown if the Execution failed.

Review comment:
       Execution -> execution.




----------------------------------------------------------------
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] pratyakshsharma commented on a change in pull request #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
pratyakshsharma commented on a change in pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#discussion_r466834745



##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/dag/scheduler/DagScheduler.java
##########
@@ -48,6 +51,11 @@ public DagScheduler(WorkflowDag workflowDag, HoodieTestSuiteWriter hoodieTestSui
     this.executionContext = new ExecutionContext(null, hoodieTestSuiteWriter, deltaGenerator);
   }
 
+  /**
+   * Method to start execute workflowDags.

Review comment:
       execute -> executing.




----------------------------------------------------------------
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] Mathieu1124 commented on pull request #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
Mathieu1124 commented on pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#issuecomment-667685860


   > > Hi @yanghua , please take a look when free :)
   > 
   > There is a conflict file. Pls resolve it then ping me. tks.
   
   @yanghua thanks for your review, it is ready 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 #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
wangxianghu commented on a change in pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#discussion_r478423255



##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/generator/GenericRecordFullPayloadGenerator.java
##########
@@ -43,22 +44,39 @@
  */
 public class GenericRecordFullPayloadGenerator implements Serializable {
 
-  public static final int DEFAULT_PAYLOAD_SIZE = 1024 * 10; // 10 KB
+  /**
+   * 10 KB.
+   */
+  public static final int DEFAULT_PAYLOAD_SIZE = 1024 * 10;
   private static Logger log = LoggerFactory.getLogger(GenericRecordFullPayloadGenerator.class);
   protected final Random random = new Random();
-  // The source schema used to generate a payload
+  /**
+   * The source schema used to generate a payload.

Review comment:
       > Why we should change these comment styles for fields?
   
   my bad,  that`s the coding guidelines of Alibaba. 
   rolled back already :)




----------------------------------------------------------------
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] Mathieu1124 commented on pull request #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
Mathieu1124 commented on pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#issuecomment-667621169


   Hi @yanghua , please take a look when free :)


----------------------------------------------------------------
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 #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#discussion_r478341019



##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/configuration/DeltaConfig.java
##########
@@ -35,7 +35,13 @@
  */
 public class DeltaConfig implements Serializable {
 
+  /**
+   * Output destination type.

Review comment:
       IMO, we do not need this comment.

##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/generator/GenericRecordFullPayloadGenerator.java
##########
@@ -43,22 +44,39 @@
  */
 public class GenericRecordFullPayloadGenerator implements Serializable {
 
-  public static final int DEFAULT_PAYLOAD_SIZE = 1024 * 10; // 10 KB
+  /**
+   * 10 KB.
+   */
+  public static final int DEFAULT_PAYLOAD_SIZE = 1024 * 10;
   private static Logger log = LoggerFactory.getLogger(GenericRecordFullPayloadGenerator.class);
   protected final Random random = new Random();
-  // The source schema used to generate a payload
+  /**
+   * The source schema used to generate a payload.

Review comment:
       Why we should change these comment styles for fields?

##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/configuration/DeltaConfig.java
##########
@@ -35,7 +35,13 @@
  */
 public class DeltaConfig implements Serializable {
 
+  /**
+   * Output destination type.
+   */
   private final DeltaOutputMode deltaOutputMode;
+  /**
+   * Input data type.

Review comment:
       ditto

##########
File path: hudi-integ-test/src/main/java/org/apache/hudi/integ/testsuite/generator/GenericRecordFullPayloadGenerator.java
##########
@@ -43,22 +44,39 @@
  */
 public class GenericRecordFullPayloadGenerator implements Serializable {
 
-  public static final int DEFAULT_PAYLOAD_SIZE = 1024 * 10; // 10 KB
+  /**
+   * 10 KB.

Review comment:
       we may not change this.




----------------------------------------------------------------
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] codecov-commenter commented on pull request #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#issuecomment-667682599


   # [Codecov](https://codecov.io/gh/apache/hudi/pull/1900?src=pr&el=h1) Report
   > Merging [#1900](https://codecov.io/gh/apache/hudi/pull/1900?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hudi/commit/30dcd5cf06c3f48bbdde1e18403ff7d0556ca511&el=desc) will **decrease** coverage by `0.69%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/hudi/pull/1900/graphs/tree.svg?width=650&height=150&src=pr&token=VTTXabwbs2)](https://codecov.io/gh/apache/hudi/pull/1900?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1900      +/-   ##
   ============================================
   - Coverage     61.38%   60.69%   -0.70%     
   + Complexity     3799     3758      -41     
   ============================================
     Files           458      458              
     Lines         19573    19573              
     Branches       1959     1959              
   ============================================
   - Hits          12015    11879     -136     
   - Misses         6744     6892     +148     
   + Partials        814      802      -12     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | #hudicli | `67.63% <ø> (-1.86%)` | `1505.00 <ø> (-42.00)` | |
   | #hudiclient | `76.73% <ø> (-2.49%)` | `1315.00 <ø> (-42.00)` | |
   | #hudicommon | `55.07% <ø> (+0.04%)` | `1518.00 <ø> (+1.00)` | |
   | #hudihadoopmr | `38.99% <ø> (ø)` | `163.00 <ø> (ø)` | |
   | #hudihivesync | `72.01% <ø> (ø)` | `121.00 <ø> (ø)` | |
   | #hudispark | `49.24% <ø> (ø)` | `124.00 <ø> (ø)` | |
   | #huditimelineservice | `63.47% <ø> (ø)` | `47.00 <ø> (ø)` | |
   | #hudiutilities | `74.12% <ø> (ø)` | `280.00 <ø> (ø)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/hudi/pull/1900?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...n/java/org/apache/hudi/index/hbase/HBaseIndex.java](https://codecov.io/gh/apache/hudi/pull/1900/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW5kZXgvaGJhc2UvSEJhc2VJbmRleC5qYXZh) | `22.00% <0.00%> (-59.81%)` | `8.00% <0.00%> (-34.00%)` | |
   | [.../index/hbase/DefaultHBaseQPSResourceAllocator.java](https://codecov.io/gh/apache/hudi/pull/1900/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW5kZXgvaGJhc2UvRGVmYXVsdEhCYXNlUVBTUmVzb3VyY2VBbGxvY2F0b3IuamF2YQ==) | `55.55% <0.00%> (-44.45%)` | `3.00% <0.00%> (-2.00%)` | |
   | [...org/apache/hudi/config/HoodieHBaseIndexConfig.java](https://codecov.io/gh/apache/hudi/pull/1900/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29uZmlnL0hvb2RpZUhCYXNlSW5kZXhDb25maWcuamF2YQ==) | `39.18% <0.00%> (-5.41%)` | `2.00% <0.00%> (ø%)` | |
   | [...java/org/apache/hudi/config/HoodieWriteConfig.java](https://codecov.io/gh/apache/hudi/pull/1900/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29uZmlnL0hvb2RpZVdyaXRlQ29uZmlnLmphdmE=) | `77.25% <0.00%> (-1.45%)` | `88.00% <0.00%> (-4.00%)` | |
   | [...java/org/apache/hudi/client/HoodieWriteClient.java](https://codecov.io/gh/apache/hudi/pull/1900/diff?src=pr&el=tree#diff-aHVkaS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpZW50L0hvb2RpZVdyaXRlQ2xpZW50LmphdmE=) | `75.86% <0.00%> (-0.87%)` | `46.00% <0.00%> (-1.00%)` | |
   | [...ache/hudi/common/fs/inline/InMemoryFileSystem.java](https://codecov.io/gh/apache/hudi/pull/1900/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2ZzL2lubGluZS9Jbk1lbW9yeUZpbGVTeXN0ZW0uamF2YQ==) | `89.65% <0.00%> (+10.34%)` | `16.00% <0.00%> (+1.00%)` | |
   


----------------------------------------------------------------
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 #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#issuecomment-667674515


   > Hi @yanghua , please take a look when free :)
   
   There is a conflict file. Pls resolve it then ping me. tks.


----------------------------------------------------------------
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] Mathieu1124 commented on pull request #1900: [HUDI-531]Add java doc for hudi test suite general classes

Posted by GitBox <gi...@apache.org>.
Mathieu1124 commented on pull request #1900:
URL: https://github.com/apache/hudi/pull/1900#issuecomment-667773592


   > 
   > 
   > @Mathieu1124 Thanks for your contribution. Left some comments. IMO, you should add more information to the doc of the classes.
   
   @yanghua  Thanks for your detailed review, I will enrich and improve the docs.


----------------------------------------------------------------
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