You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/07/30 07:34:13 UTC

[GitHub] [hadoop-ozone] rakeshadr commented on a change in pull request #1264: HDDS-4035. Update logs of HadoopDirGenerator.

rakeshadr commented on a change in pull request #1264:
URL: https://github.com/apache/hadoop-ozone/pull/1264#discussion_r462716072



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/BaseFreonGenerator.java
##########
@@ -301,6 +301,15 @@ public void printReport() {
     messages.forEach(print);
   }
 
+  public void print(String s){

Review comment:
       Latest patch looks good. Adding few minor comments:
   
   Please add javadoc and change the parameter name 's' to 'msg', just to improve more readable.

##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/HadoopDirTreeGenerator.java
##########
@@ -97,17 +97,25 @@
 
   @Override
   public Void call() throws Exception {
-
-    init();
-    OzoneConfiguration configuration = createOzoneConfiguration();
-    fileSystem = FileSystem.get(URI.create(rootPath), configuration);
-
-    contentGenerator = new ContentGenerator(fileSizeInBytes, bufferSize);
-    timer = getMetrics().timer("file-create");
-
-    runTests(this::createDir);
+    String s;
+    if (depth <= 0) {
+      s = "Invalid depth value, depth value should be greater than zero!";
+      print(s);
+    } else if (span <= 0) {
+      s = "Invalid span value, span value should be greater than zero!";
+      print(s);
+    } else {
+      init();
+      OzoneConfiguration configuration = createOzoneConfiguration();
+      fileSystem = FileSystem.get(URI.create(rootPath), configuration);
+
+      contentGenerator = new ContentGenerator(fileSizeInBytes, bufferSize);
+      timer = getMetrics().timer("file-create");
+
+      runTests(this::createDir);
+      return null;

Review comment:
       minor comment: please remove 'return null' inside the else block as below return is sufficient.

##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/BaseFreonGenerator.java
##########
@@ -301,6 +301,15 @@ public void printReport() {
     messages.forEach(print);
   }
 
+  public void print(String s){

Review comment:
       Latest patch looks good. Adding few minor comments:
   
   Please add javadoc and change the parameter name 's' to 'msg', just to make it more readable.

##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/BaseFreonGenerator.java
##########
@@ -301,6 +301,19 @@ public void printReport() {
     messages.forEach(print);
   }
 
+  /**
+   * Print out the messages for HadoopNestedDirGenerator

Review comment:
       As this is base class, please remove the inherited class details here.
   
   you can write something like,
   "Print out reports with the given message."
   
   




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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org