You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/06/18 15:50:00 UTC

[GitHub] [hbase-operator-tools] clarax opened a new pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

clarax opened a new pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69


   …a list of region names/table names/namespaces


----------------------------------------------------------------
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] [hbase-operator-tools] clarax edited a comment on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax edited a comment on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1058275404


   When I implemented for assigns and unassigns, I used -i after command. But once I tried to implement for all commands, I realized it could be a global flag. Making it a flag for individual command would have to deal with different parsing and validation for every single command which requires significant refactoring, code duplication and much more unit tests.


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] clarax commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1055851862


   @wchevreuil @ndimiduk Thank you for the feedback. Sorry I shelved this for other priorities. I am back to finish this.  The changes are:
   1. added a common -i option so any command that takes a list of argument can take a list of input files.
   2. updated the -h message and README.
   3. added a unit test for the generic option and test this option for assigns and unassigns. For other commands, since they all share the same code path, I don't see a need to add more ut. The existing ut for other command don't touch generic options either.
   4. for commands that take two arguments(table|resgion state), I will open a separate jira and pr for taking the pairs in a list of files because that involves changing the interface of calls to server. 


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1054882093


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 37s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 48s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 11s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 12s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 10s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 10s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m  5s |  hbase-hbck2: The patch generated 9 new + 0 unchanged - 0 fixed = 9 total (was 0)  |
   | -1 :x: |  whitespace  |   0m  0s |  The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply  |
   | +1 :green_heart: |  javadoc  |   0m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   3m 39s |  hbase-hbck2 in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m  6s |  The patch does not generate ASF License warnings.  |
   |  |   |   7m 19s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/2/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/69 |
   | Optional Tests | dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 885f8905d9c9 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / b7d9fc8 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   | checkstyle | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/2/artifact/yetus-precommit-check/output/diff-checkstyle-hbase-hbck2.txt |
   | whitespace | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/2/artifact/yetus-precommit-check/output/whitespace-eol.txt |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/2/testReport/ |
   | Max. process+thread count | 1215 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/2/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] clarax edited a comment on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax edited a comment on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1055851862


   @wchevreuil @ndimiduk Thank you for the feedback. Sorry I shelved this for other priorities. I am back to finish this.  The changes are:
   1. added a common -i option so any command that takes a list of argument can take a list of input files.
   2. updated the -h message and README.
   3. added a unit test for the generic option and test this option for assigns and unassigns. For other commands, since they all share the same code path, I don't see a need to add more ut. The existing ut for other command don't touch generic options either.
   4. for commands that take two arguments(table|resgion state), I opened a separate jira and pr for taking the pairs in a list of files because that involves changing the interface of calls to server.  https://issues.apache.org/jira/browse/HBASE-26785


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1056188435


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 46s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 11s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  6s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 10s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 10s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m  5s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  15m 22s |  hbase-hbck2 in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m  5s |  The patch does not generate ASF License warnings.  |
   |  |   |  18m 26s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hbase.TestHBCKCommandLineParsing |
   |   | hbase.TestHBCK2 |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/7/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/69 |
   | Optional Tests | dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux d975cbd2e4f5 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / b7d9fc8 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   | unit | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/7/artifact/yetus-precommit-check/output/patch-unit-hbase-hbck2.txt |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/7/testReport/ |
   | Max. process+thread count | 1220 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/7/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] ankitsinghal commented on a change in pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
ankitsinghal commented on a change in pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#discussion_r830549829



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -709,17 +766,24 @@ private static void usageSetTableState(PrintWriter writer) {
     writer.println("   An example making table name 'user' ENABLED:");
     writer.println("     $ HBCK2 setTableState users ENABLED");
     writer.println("   Returns whatever the previous table state was.");
+    writer.println("   If -i or --inputFiles is specified, pass one or more input file names.");
+    writer.println("   Each file contains <TABLENAME> <STATE>, one pair per line.);" +
+            "For example:");
+    writer.println("     $ HBCK2 -i " + SET_TABLE_STATE + " fileName1 fileName2");
   }
 
   private static void usageScheduleRecoveries(PrintWriter writer) {
-    writer.println(" " + SCHEDULE_RECOVERIES + " <SERVERNAME>...");
+    writer.println(" " + SCHEDULE_RECOVERIES + " <SERVERNAME|INPUTFILE_FOR_SERVERNAMES>...");

Review comment:
       ```suggestion
       writer.println(" " + SCHEDULE_RECOVERIES + " <Either SERVERNAME OR INPUTFILE_FOR_SERVERNAMES>...");
   ```
   As HBase expects | in some format like coprocessor so if we can be more descriptive that would be good.

##########
File path: hbase-hbck2/README.md
##########
@@ -280,6 +281,9 @@ Command:
    of what a userspace encoded region name looks like. For example:
      $ HBCK2 unassign 1588230740 de00010733901a05f5a2a3a382e27dd4
    Returns the pid(s) of the created UnassignProcedure(s) or -1 if none.
+   If -i or --inputFiles is specified, pass one or more input file names.

Review comment:
       Agreed , I think we need to define "-i " option well (and remove --inputFiles to avoid confusion), that it just alter the way the args are expected by the command, if "-i" is used and a file is not passed, a user should see an error 
   
   OR
   
   your earlier suggestion make it local to every command so that it can be defined as "extraRegionsInMeta -i <filename1> <filename2> " instead of global "-i extraRegionsInMeta <filename1> <filename2>" . 
   
   I think if it is well defined both options are good but we can choose one and proceed, I'm more inclining also towards "local" option as we anyways have to describe them for each command. let me know your thoughts so that we can close on this.
   
   
   

##########
File path: hbase-hbck2/README.md
##########
@@ -91,7 +91,9 @@ The above command with no options or arguments passed will dump out the _HBCK2_
 usage: HBCK2 [OPTIONS] COMMAND <ARGS>
 Options:
  -d,--debug                                       run with debug output
+ -i, --inputfiles                                 take one or more files to read the args from

Review comment:
       Nick is suggesting this @clarax 
   ```suggestion
    -i, --inputFiles                                 take one or more files to read the args from
   ```

##########
File path: hbase-hbck2/README.md
##########
@@ -173,6 +181,9 @@ Command:
      $ HBCK2 extraRegionsInMeta default:table_1 ns1
    Returns list of extra regions for each table passed as parameter, or
    for each table on namespaces specified as parameter.
+   If -i or --inputFiles is specified, pass one or more input file names.
+   Each file contains <NAMESPACE|NAMESPACE:TABLENAME>, one per line. For example:

Review comment:
       If you mean by either NAMESPACE or NAMESPACE:TABLENAME, as | looks like a part of a format.
   ```suggestion
      Each file contains <NAMESPACE or NAMESPACE:TABLENAME>, one per line. For example:
   ```

##########
File path: hbase-hbck2/README.md
##########
@@ -137,12 +138,13 @@ Command:
    Returns the pid(s) of the created AssignProcedure(s) or -1 if none.
    If -i or --inputFiles is specified, pass one or more input file names.
    Each file contains encoded region names, one per line. For example:
-     $ HBCK2 assigns -i fileName1 fileName2
+     $ HBCK2 -i assigns fileName1 fileName2

Review comment:
       I think the idea here is that "-i" changes how the args are expected by the command so instead of they being listed as on console, the command will accept the file instead but values/format would be different for different commands.

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -733,7 +797,7 @@ private static void usageRecoverUnknown(PrintWriter writer) {
   }
 
   private static void usageUnassigns(PrintWriter writer) {
-    writer.println(" " + UNASSIGNS + " <ENCODED_REGIONNAME>...");
+    writer.println(" " + UNASSIGNS + " <ENCODED_REGIONNAME|INPUTFILE_FOR_REGIONNAMES>...");

Review comment:
       ```suggestion
       writer.println(" " + UNASSIGNS + " <either ENCODED_REGIONNAME or INPUTFILE_FOR_REGIONNAMES>...");
   ```

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -892,17 +965,32 @@ private int doCommandLine(CommandLine commandLine, Options options) throws IOExc
       // Case handlers all have same format. Check first that the server supports
       // the feature FIRST, then move to process the command.
       case SET_TABLE_STATE:
-        if (commands.length < 3) {
-          showErrorMessage(command + " takes tablename and state arguments: e.g. user ENABLED");
-          return EXIT_FAILURE;
+        if (getFromFile){
+          if (commands.length < 2) {
+            showErrorMessage(command + " takes a list of file names");
+            return EXIT_FAILURE;
+          }
+          List<String> argList = getFromFiles(Arrays.asList(purgeFirst(commands)));
+          try (ClusterConnection connection = connect(); Hbck hbck = connection.getHbck()) {
+            checkFunctionSupported(connection, command);
+            for (String line : argList) {
+              String[] args = line.split("\\s+");
+              System.out.println(setTableState(hbck, args));
+            }
+          }
         }
-        try (ClusterConnection connection = connect(); Hbck hbck = connection.getHbck()) {
-          checkFunctionSupported(connection, command);
-          System.out.println(setTableState(hbck, TableName.valueOf(commands[1]),
-              TableState.State.valueOf(commands[2])));
+        else {
+          if (commands.length < 3) {
+            showErrorMessage(SET_TABLE_STATE +
+                    " takes tablename and state arguments: e.g. user ENABLED");
+            return EXIT_FAILURE;
+          }
+          try (ClusterConnection connection = connect(); Hbck hbck = connection.getHbck()) {
+            checkFunctionSupported(connection, command);
+            System.out.println(setTableState(hbck, purgeFirst(commands)));
+          }

Review comment:
       can this be combined to avoid redundant code?

##########
File path: hbase-hbck2/README.md
##########
@@ -91,7 +91,9 @@ The above command with no options or arguments passed will dump out the _HBCK2_
 usage: HBCK2 [OPTIONS] COMMAND <ARGS>
 Options:
  -d,--debug                                       run with debug output
+ -i, --inputfiles                                 take one or more files to read the args from
  -h,--help                                        output this help message
+ -i,--inputFiles                                  take one or more encoded region names

Review comment:
       Duplicate option defined?

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -944,31 +1032,29 @@ private int doCommandLine(CommandLine commandLine, Options options) throws IOExc
         break;
 
       case SET_REGION_STATE:
-        if (commands.length < 3) {
-          showErrorMessage(command + " takes region encoded name and state arguments: e.g. "
-              + "35f30b0ce922c34bf5c284eff33ba8b3 CLOSING");
-          return EXIT_FAILURE;
-        }
-        RegionState.State state = RegionState.State.valueOf(commands[2]);
-
-        int replicaId = 0;
-        String region = commands[1];
-        int separatorIndex = commands[1].indexOf(",");
-        if (separatorIndex > 0) {
-          region = commands[1].substring(0, separatorIndex);
-          replicaId = Integer.getInteger(commands[1].substring(separatorIndex + 1));
-        }
-
-        if (replicaId > 0) {
-          System.out.println("Change state for replica reigon " + replicaId  +
-                  " for primary region " + region);
-        }
-
-        try (ClusterConnection connection = connect()) {
-          checkHBCKSupport(connection, command);
-          return setRegionState(connection, region, replicaId, state);
+        if (getFromFile) {
+          if (commands.length < 2) {
+            showErrorMessage(command + " takes a list of file names");
+            return EXIT_FAILURE;
+          }
+          List<String> argList = getFromFiles(Arrays.asList(purgeFirst(commands)));
+          try (ClusterConnection connection = connect()) {
+            checkHBCKSupport(connection, command);
+            for (String line : argList) {
+              String[] args = formatSetRegionStateCommand(line.split("\\s+"));
+              if (setRegionState(connection, args) == EXIT_FAILURE) {
+                showErrorMessage(command + " failed to set " + args);
+              }
+            }
+          }
+          break;
+        } else {
+          String[] args = formatSetRegionStateCommand(purgeFirst(commands));
+          try (ClusterConnection connection = connect()) {
+            checkHBCKSupport(connection, command);
+            return setRegionState(connection, args);
+          }

Review comment:
       Can this be combined to avoid redundant code? 




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] wchevreuil commented on a change in pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#discussion_r442869982



##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -203,8 +204,8 @@ int setRegionState(ClusterConnection connection, String region,
     Map<TableName,List<Path>> report;
     try (final FsRegionsMetaRecoverer fsRegionsMetaRecoverer =
         new FsRegionsMetaRecoverer(this.conf)) {
-      report = fsRegionsMetaRecoverer.reportTablesMissingRegions(
-        formatNameSpaceTableParam(nameSpaceOrTable));
+      report = fsRegionsMetaRecoverer.reportTablesMissingRegions(getFromArgsOrFiles(

Review comment:
       Missing UT and updates to `reportMissingRegionsInMeta` command description (as it now accepts list of files)

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -297,21 +292,8 @@ int setRegionState(ClusterConnection connection, String region,
       return null;
     }
     boolean overrideFlag = commandLine.hasOption(override.getOpt());
-
     List<String> argList = commandLine.getArgList();
-    if (!commandLine.hasOption(inputFile.getOpt())) {
-      return hbck.assigns(argList, overrideFlag);
-    }
-    List<String> assignmentList = new ArrayList<>();
-    for (String filePath : argList) {
-      try (InputStream fileStream = new FileInputStream(filePath)){
-        LineIterator it = IOUtils.lineIterator(fileStream, "UTF-8");
-        while (it.hasNext()) {
-          assignmentList.add(it.nextLine().trim());
-        }
-      }
-    }
-    return hbck.assigns(assignmentList, overrideFlag);
+    return hbck.assigns(this.getFromArgsOrFiles(argList), overrideFlag);

Review comment:
       Missing UT and updates to `setRegionState` command description (as it now accepts list of files)

##########
File path: hbase-hbck2/README.md
##########
@@ -280,6 +281,9 @@ Command:
    of what a userspace encoded region name looks like. For example:
      $ HBCK2 unassign 1588230740 de00010733901a05f5a2a3a382e27dd4
    Returns the pid(s) of the created UnassignProcedure(s) or -1 if none.
+   If -i or --inputFiles is specified, pass one or more input file names.

Review comment:
       We should modify the command params description on line #274 to:
   
   `unassigns <ENCODED_REGIONNAME|-i INPUT_FILES>`
   
   

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -265,16 +266,12 @@ int setRegionState(ClusterConnection connection, String region,
     return result;
   }
 
-  private List<String> formatNameSpaceTableParam(String... nameSpaceOrTable) {
-    return nameSpaceOrTable != null ? Arrays.asList(nameSpaceOrTable) : null;
-  }
-
   List<Future<List<String>>> addMissingRegionsInMetaForTables(String...
       nameSpaceOrTable) throws IOException {
     try (final FsRegionsMetaRecoverer fsRegionsMetaRecoverer =
       new FsRegionsMetaRecoverer(this.conf)) {
-      return fsRegionsMetaRecoverer.addMissingRegionsInMetaForTables(
-        formatNameSpaceTableParam(nameSpaceOrTable));
+      return fsRegionsMetaRecoverer.addMissingRegionsInMetaForTables(getFromArgsOrFiles(

Review comment:
       Missing UT and updates to `addFsRegionsMissingInMeta` command description (as it now accepts list of files)

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -228,7 +229,7 @@ int setRegionState(ClusterConnection connection, String region,
     Map<TableName, List<String>> result = new HashMap<>();
     try (final FsRegionsMetaRecoverer fsRegionsMetaRecoverer =
       new FsRegionsMetaRecoverer(this.conf)) {
-      List<String> namespacesTables = formatNameSpaceTableParam(commandLine.getArgs());
+      List<String> namespacesTables = getFromArgsOrFiles(formatNameSpaceTableParam(commandLine.getArgs()));

Review comment:
       Missing UT and updates to `extraRegionsInMeta` command description (as it now accepts list of files)




----------------------------------------------------------------
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] [hbase-operator-tools] clarax edited a comment on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax edited a comment on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1058275404


   When I implemented for assigns and unassigns, I used -i after command. But once I tried to implement for all commands, I realized it could be a global flag. Making it a flag for individual command would have to deal with different parsing and validation for every single command which requires significant refactoring, code duplication and much more unit tests.
   
   But if we foresee adding more options to individual commands down the road, it is worth to standardize various command parsing/validation logic.


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1056035021


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | -1 :x: |  mvninstall  |   2m 56s |  root in master failed.  |
   | +1 :green_heart: |  compile  |   0m 34s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  8s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 12s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m  4s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  16m 18s |  hbase-hbck2 in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m  6s |  The patch does not generate ASF License warnings.  |
   |  |   |  21m 25s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hbase.TestHBCK2 |
   |   | hbase.TestHBCKCommandLineParsing |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/6/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/69 |
   | Optional Tests | dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux a7d4702ae314 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / b7d9fc8 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   | mvninstall | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/6/artifact/yetus-precommit-check/output/branch-mvninstall-root.txt |
   | unit | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/6/artifact/yetus-precommit-check/output/patch-unit-hbase-hbck2.txt |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/6/testReport/ |
   | Max. process+thread count | 1201 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/6/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1057499022


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  8s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 54s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 10s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 12s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 10s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 10s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m  4s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   3m 35s |  hbase-hbck2 in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m  6s |  The patch does not generate ASF License warnings.  |
   |  |   |   6m 47s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/8/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/69 |
   | Optional Tests | dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 67c579d8c97d 5.4.0-1047-aws #49~18.04.1-Ubuntu SMP Wed Apr 28 23:08:58 UTC 2021 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / b7d9fc8 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/8/testReport/ |
   | Max. process+thread count | 1259 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/8/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-675053254


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | -1 :x: |  mvninstall  |   0m 18s |  root in master failed.  |
   | -1 :x: |  compile  |   0m 17s |  hbase-hbck2 in master failed.  |
   | -1 :x: |  checkstyle  |   0m 16s |  The patch fails to run checkstyle in hbase-hbck2  |
   | -1 :x: |  javadoc  |   0m 18s |  hbase-hbck2 in master failed.  |
   ||| _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   0m 18s |  hbase-hbck2 in the patch failed.  |
   | -1 :x: |  compile  |   0m 17s |  hbase-hbck2 in the patch failed.  |
   | -1 :x: |  javac  |   0m 17s |  hbase-hbck2 in the patch failed.  |
   | -1 :x: |  checkstyle  |   0m 15s |  The patch fails to run checkstyle in hbase-hbck2  |
   | +1 :green_heart: |  whitespace  |   0m  1s |  The patch has no whitespace issues.  |
   | -1 :x: |  javadoc  |   0m 17s |  hbase-hbck2 in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   0m 18s |  hbase-hbck2 in the patch failed.  |
   | +0 :ok: |  asflicense  |   0m 17s |  ASF License check generated no output?  |
   |  |   |   3m 52s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/69 |
   | Optional Tests | dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 932ec5fe3be5 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / 87878aa |
   | Default Java | Oracle Corporation-1.8.0_265-b01 |
   | mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/branch-mvninstall-root.txt |
   | compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/branch-compile-hbase-hbck2.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/buildtool-branch-checkstyle-hbase-hbck2.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/branch-javadoc-hbase-hbck2.txt |
   | mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/patch-mvninstall-hbase-hbck2.txt |
   | compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/patch-compile-hbase-hbck2.txt |
   | javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/patch-compile-hbase-hbck2.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/buildtool-patch-checkstyle-hbase-hbck2.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/patch-javadoc-hbase-hbck2.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/patch-unit-hbase-hbck2.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/testReport/ |
   | Max. process+thread count | 29 (vs. ulimit of 1000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-Connectors-PreCommit/job/PR-69/1/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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] [hbase-operator-tools] wchevreuil commented on a change in pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#discussion_r443493638



##########
File path: hbase-hbck2/README.md
##########
@@ -280,6 +281,9 @@ Command:
    of what a userspace encoded region name looks like. For example:
      $ HBCK2 unassign 1588230740 de00010733901a05f5a2a3a382e27dd4
    Returns the pid(s) of the created UnassignProcedure(s) or -1 if none.
+   If -i or --inputFiles is specified, pass one or more input file names.

Review comment:
       We need still to update README for each of the modified commands. For example, _assigns_ descriptions reads:
   `assigns [OPTIONS] <ENCODED_REGIONNAME/INPUTFILES_FOR_REGIONNAMES>...`
   
   But _extraRegionsInMeta_ still reads:
   `extraRegionsInMeta <NAMESPACE|NAMESPACE:TABLENAME>...` 
   
   However, command usage for modified commands do mention the correct parameters, so we should make README consistent.

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -357,23 +363,24 @@ int setRegionState(ClusterConnection connection, String region,
     if (commandLine.hasOption(wait.getOpt())) {
       lockWait = Integer.parseInt(commandLine.getOptionValue(wait.getOpt()));
     }
-    String[] pidStrs = commandLine.getArgs();
-    if (pidStrs == null || pidStrs.length <= 0) {
+    List<String> pidStrs = getFromArgsOrFiles(commandLine.getArgList());

Review comment:
       Requires UT for the new parameter option.

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -228,7 +229,7 @@ int setRegionState(ClusterConnection connection, String region,
     Map<TableName, List<String>> result = new HashMap<>();
     try (final FsRegionsMetaRecoverer fsRegionsMetaRecoverer =
       new FsRegionsMetaRecoverer(this.conf)) {
-      List<String> namespacesTables = formatNameSpaceTableParam(commandLine.getArgs());
+      List<String> namespacesTables = getFromArgsOrFiles(formatNameSpaceTableParam(commandLine.getArgs()));

Review comment:
       Still missing UT for when files are passed as parameter. See _TestHBCK2.testFormatFixExtraInMetaOneExtraSpecificTable_.

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -203,8 +204,8 @@ int setRegionState(ClusterConnection connection, String region,
     Map<TableName,List<Path>> report;
     try (final FsRegionsMetaRecoverer fsRegionsMetaRecoverer =
         new FsRegionsMetaRecoverer(this.conf)) {
-      report = fsRegionsMetaRecoverer.reportTablesMissingRegions(
-        formatNameSpaceTableParam(nameSpaceOrTable));
+      report = fsRegionsMetaRecoverer.reportTablesMissingRegions(getFromArgsOrFiles(

Review comment:
       Still missing UT for _reportMissingRegionsInMeta_. See _TestHBCK2.testReportMissingRegionsInMeta_. 

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -357,23 +363,24 @@ int setRegionState(ClusterConnection connection, String region,
     if (commandLine.hasOption(wait.getOpt())) {
       lockWait = Integer.parseInt(commandLine.getOptionValue(wait.getOpt()));
     }
-    String[] pidStrs = commandLine.getArgs();
-    if (pidStrs == null || pidStrs.length <= 0) {
+    List<String> pidStrs = getFromArgsOrFiles(commandLine.getArgList());
+    if (pidStrs == null || pidStrs.size() <= 0) {
       showErrorMessage("No pids supplied.");
       return null;
     }
     boolean overrideFlag = commandLine.hasOption(override.getOpt());
     boolean recursiveFlag = commandLine.hasOption(recursive.getOpt());
-    List<Long> pids = Arrays.stream(pidStrs).map(Long::valueOf).collect(Collectors.toList());
+    List<Long> pids = pidStrs.stream().map(Long::valueOf).collect(Collectors.toList());
     try (ClusterConnection connection = connect(); Hbck hbck = connection.getHbck()) {
       checkFunctionSupported(connection, BYPASS);
       return hbck.bypassProcedure(pids, lockWait, overrideFlag, recursiveFlag);
     }
   }
 
   List<Long> scheduleRecoveries(Hbck hbck, String[] args) throws IOException {
+    List<String> arglist = getFromArgsOrFiles(Arrays.asList(args));

Review comment:
       Requires UT for the new parameter option.

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -265,16 +266,12 @@ int setRegionState(ClusterConnection connection, String region,
     return result;
   }
 
-  private List<String> formatNameSpaceTableParam(String... nameSpaceOrTable) {
-    return nameSpaceOrTable != null ? Arrays.asList(nameSpaceOrTable) : null;
-  }
-
   List<Future<List<String>>> addMissingRegionsInMetaForTables(String...
       nameSpaceOrTable) throws IOException {
     try (final FsRegionsMetaRecoverer fsRegionsMetaRecoverer =
       new FsRegionsMetaRecoverer(this.conf)) {
-      return fsRegionsMetaRecoverer.addMissingRegionsInMetaForTables(
-        formatNameSpaceTableParam(nameSpaceOrTable));
+      return fsRegionsMetaRecoverer.addMissingRegionsInMetaForTables(getFromArgsOrFiles(

Review comment:
       Still missing UT for the condition where files are passed as input. See _TestHBCK2.testAddMissingRegionsInMetaForTables_




----------------------------------------------------------------
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] [hbase-operator-tools] clarax commented on a change in pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#discussion_r443257734



##########
File path: hbase-hbck2/README.md
##########
@@ -280,6 +281,9 @@ Command:
    of what a userspace encoded region name looks like. For example:
      $ HBCK2 unassign 1588230740 de00010733901a05f5a2a3a382e27dd4
    Returns the pid(s) of the created UnassignProcedure(s) or -1 if none.
+   If -i or --inputFiles is specified, pass one or more input file names.

Review comment:
       I added -i as a utility option for all commands that take a list of argument. So the syntax is 
   hbck -i command inputfiles




----------------------------------------------------------------
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] [hbase-operator-tools] clarax commented on a change in pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#discussion_r443257734



##########
File path: hbase-hbck2/README.md
##########
@@ -280,6 +281,9 @@ Command:
    of what a userspace encoded region name looks like. For example:
      $ HBCK2 unassign 1588230740 de00010733901a05f5a2a3a382e27dd4
    Returns the pid(s) of the created UnassignProcedure(s) or -1 if none.
+   If -i or --inputFiles is specified, pass one or more input file names.

Review comment:
       I added -i as an option for all commands that take a list of argument. So the sync is 
   hbck -i command inputfiles




----------------------------------------------------------------
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] [hbase-operator-tools] Apache-HBase commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1055052626


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 41s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 46s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 10s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  8s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 10s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 10s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m  5s |  hbase-hbck2: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   3m 41s |  hbase-hbck2 in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m  6s |  The patch does not generate ASF License warnings.  |
   |  |   |   7m 23s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hbase.TestHBCKCommandLineParsing |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/3/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/69 |
   | Optional Tests | dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 5c28f1fd1c68 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / b7d9fc8 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   | checkstyle | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/3/artifact/yetus-precommit-check/output/diff-checkstyle-hbase-hbck2.txt |
   | unit | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/3/artifact/yetus-precommit-check/output/patch-unit-hbase-hbck2.txt |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/3/testReport/ |
   | Max. process+thread count | 1222 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/3/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1055971294


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 36s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   2m 28s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 10s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  8s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m  9s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m  4s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   3m 28s |  hbase-hbck2 in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m  6s |  The patch does not generate ASF License warnings.  |
   |  |   |   7m 42s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/5/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/69 |
   | Optional Tests | dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 3ab23ae7519a 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / b7d9fc8 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/5/testReport/ |
   | Max. process+thread count | 1211 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/5/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] ndimiduk commented on a change in pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#discussion_r818703794



##########
File path: hbase-hbck2/README.md
##########
@@ -225,10 +238,12 @@ Command:
     -f, --fix    fix any replication issues found.
    Looks for undeleted replication queues and deletes them if passed the
    '--fix' option. Pass a table name to check for replication barrier and
-   purge if '--fix'.
+   purge if '--fix'. If -i or --inputFiles is specified, pass one or more input file names.
+   Each file contains <TABLENAME>, one per line. For example:
+     $ HBCK2 -i replication fileName1 fileName2
 
  reportMissingRegionsInMeta <NAMESPACE|NAMESPACE:TABLENAME>...
-   To be used when regions missing from hbase:meta but directories
+    To be used when regions missing from hbase:meta but directories

Review comment:
       Another unintentional whitespace change?

##########
File path: hbase-hbck2/README.md
##########
@@ -137,19 +142,22 @@ Command:
    Returns the pid(s) of the created AssignProcedure(s) or -1 if none.
    If -i or --inputFiles is specified, pass one or more input file names.
    Each file contains encoded region names, one per line. For example:
-     $ HBCK2 assigns -i fileName1 fileName2
+     $ HBCK2 -i assigns fileName1 fileName2
  bypass [OPTIONS] <PID>...
    Options:
     -o,--override   override if procedure is running/stuck
     -r,--recursive  bypass parent and its children. SLOW! EXPENSIVE!
     -w,--lockWait   milliseconds to wait before giving up; default=1
-   Pass one (or more) procedure 'pid's to skip to procedure finish. Parent
+  Pass one (or more) procedure 'pid's to skip to procedure finish. Parent

Review comment:
       Unintentional whitespace change?

##########
File path: hbase-hbck2/README.md
##########
@@ -280,6 +281,9 @@ Command:
    of what a userspace encoded region name looks like. For example:
      $ HBCK2 unassign 1588230740 de00010733901a05f5a2a3a382e27dd4
    Returns the pid(s) of the created UnassignProcedure(s) or -1 if none.
+   If -i or --inputFiles is specified, pass one or more input file names.

Review comment:
       This new behavior is surprising to me. It's surprising because the `-i` is accepted as a global flag but it's arguments come after the subcommand. That's not a common feature in command line utilities that I'm familiar with. I'm not convinced this is a good behavior.

##########
File path: hbase-hbck2/README.md
##########
@@ -137,12 +138,13 @@ Command:
    Returns the pid(s) of the created AssignProcedure(s) or -1 if none.
    If -i or --inputFiles is specified, pass one or more input file names.
    Each file contains encoded region names, one per line. For example:
-     $ HBCK2 assigns -i fileName1 fileName2
+     $ HBCK2 -i assigns fileName1 fileName2

Review comment:
       So should this docstring be `HBCK2 -i fileName1 fileName2 assigns ...` ? Or maybe don't document `-i` here in the `assigns` subcommand because it's a "global" feature.

##########
File path: hbase-hbck2/README.md
##########
@@ -91,7 +91,9 @@ The above command with no options or arguments passed will dump out the _HBCK2_
 usage: HBCK2 [OPTIONS] COMMAND <ARGS>
 Options:
  -d,--debug                                       run with debug output
+ -i, --inputfiles                                 take one or more files to read the args from

Review comment:
       You need document the new argument only once. It looks like the camel-case version, `inputFiles` is what your parser change accepts.




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] clarax commented on a change in pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#discussion_r444562529



##########
File path: hbase-hbck2/README.md
##########
@@ -137,12 +138,13 @@ Command:
    Returns the pid(s) of the created AssignProcedure(s) or -1 if none.
    If -i or --inputFiles is specified, pass one or more input file names.
    Each file contains encoded region names, one per line. For example:
-     $ HBCK2 assigns -i fileName1 fileName2
+     $ HBCK2 -i assigns fileName1 fileName2

Review comment:
       I changed it to a utility wide option like --skip




----------------------------------------------------------------
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] [hbase-operator-tools] busbey commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
busbey commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-761120765


   @clarax still working on 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] [hbase-operator-tools] Apache-HBase commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1055883493


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  4s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m 58s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 10s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m  7s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m  7s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 11s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 10s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 10s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m  5s |  hbase-hbck2: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   3m 34s |  hbase-hbck2 in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m  5s |  The patch does not generate ASF License warnings.  |
   |  |   |   7m 47s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/4/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/69 |
   | Optional Tests | dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 9a9541eb8003 5.4.0-1025-aws #25~18.04.1-Ubuntu SMP Fri Sep 11 12:03:04 UTC 2020 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / b7d9fc8 |
   | Default Java | Oracle Corporation-1.8.0_282-b08 |
   | checkstyle | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/4/artifact/yetus-precommit-check/output/diff-checkstyle-hbase-hbck2.txt |
   |  Test Results | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/4/testReport/ |
   | Max. process+thread count | 1210 (vs. ulimit of 5000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hbase.apache.org/job/HBase-Operator-Tools-PreCommit/job/PR-69/4/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] clarax commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1058275404


   When I implemented for assigns and unassigns, I used -i after command. But once I try to implement for all commands, I realized it could be a global flag. Making it a flag for individual command would have to deal with different parsing and validation for every single command which requires significant refactoring, code duplication and much more unit tests.


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] clarax commented on a change in pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax commented on a change in pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#discussion_r818860250



##########
File path: hbase-hbck2/README.md
##########
@@ -91,7 +91,9 @@ The above command with no options or arguments passed will dump out the _HBCK2_
 usage: HBCK2 [OPTIONS] COMMAND <ARGS>
 Options:
  -d,--debug                                       run with debug output
+ -i, --inputfiles                                 take one or more files to read the args from

Review comment:
       This was per @wchevreuil 's feedback.




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] Apache-HBase commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-674974690


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 34s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +0 :ok: |  spotbugs  |   0m  0s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 58s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 16s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 11s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 11s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 18s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 16s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 16s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m  9s |  hbase-hbck2: The patch generated 9 new + 3 unchanged - 2 fixed = 12 total (was 5)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  javadoc  |   0m  8s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  18m 35s |  hbase-hbck2 in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m  8s |  The patch does not generate ASF License warnings.  |
   |  |   |  21m 58s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hbase.TestHBCK2 |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/2/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/69 |
   | Optional Tests | dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux ee302be4afae 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / 87878aa |
   | Default Java | Oracle Corporation-1.8.0_265-b01 |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/2/artifact/yetus-precommit-check/output/diff-checkstyle-hbase-hbck2.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/2/artifact/yetus-precommit-check/output/patch-unit-hbase-hbck2.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/2/testReport/ |
   | Max. process+thread count | 910 (vs. ulimit of 1000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/2/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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] [hbase-operator-tools] Apache-HBase commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-674962117


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 16s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +0 :ok: |  markdownlint  |   0m  0s |  markdownlint was not available.  |
   | +0 :ok: |  spotbugs  |   0m  1s |  spotbugs executables are not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | -1 :x: |  mvninstall  |   0m 17s |  root in master failed.  |
   | -1 :x: |  compile  |   0m 18s |  hbase-hbck2 in master failed.  |
   | -1 :x: |  checkstyle  |   0m 26s |  The patch fails to run checkstyle in hbase-hbck2  |
   | -1 :x: |  javadoc  |   0m 18s |  hbase-hbck2 in master failed.  |
   ||| _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   0m 18s |  hbase-hbck2 in the patch failed.  |
   | +1 :green_heart: |  compile  |   0m 40s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 40s |  the patch passed  |
   | -1 :x: |  checkstyle  |   0m  1s |  The patch fails to run checkstyle in hbase-hbck2  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | -1 :x: |  javadoc  |   0m  3s |  hbase-hbck2 in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   0m  3s |  hbase-hbck2 in the patch failed.  |
   | +0 :ok: |  asflicense  |   0m  2s |  ASF License check generated no output?  |
   |  |   |   4m  4s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase-operator-tools/pull/69 |
   | Optional Tests | dupname asflicense markdownlint javac javadoc unit spotbugs findbugs checkstyle compile |
   | uname | Linux 61aad65f9996 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 GNU/Linux |
   | Build tool | maven |
   | git revision | master / 87878aa |
   | Default Java | Oracle Corporation-1.8.0_265-b01 |
   | mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/branch-mvninstall-root.txt |
   | compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/branch-compile-hbase-hbck2.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/buildtool-branch-checkstyle-hbase-hbck2.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/branch-javadoc-hbase-hbck2.txt |
   | mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/patch-mvninstall-hbase-hbck2.txt |
   | checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/buildtool-patch-checkstyle-hbase-hbck2.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/patch-javadoc-hbase-hbck2.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/artifact/yetus-precommit-check/output/patch-unit-hbase-hbck2.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/testReport/ |
   | Max. process+thread count | 52 (vs. ulimit of 1000) |
   | modules | C: hbase-hbck2 U: hbase-hbck2 |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-Operator-Tools-PreCommit/job/PR-69/1/console |
   | versions | git=2.20.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
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] [hbase-operator-tools] clarax edited a comment on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax edited a comment on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1055851862


   @wchevreuil @ndimiduk Thank you for the feedback. Sorry I shelved this for other priorities. I am back to finish this.  The changes are:
   1. added a common -i option so any command that takes a list of argument can take a list of input files.
   2. updated the -h message and README.
   3. added a unit test for the generic option and test this option for assigns and unassigns. For other commands, since they all share the same code path, I don't see a need to add more ut. The existing ut for other command don't touch generic options either.
   4. for commands that take two arguments(table|resgion state), I opened a separate jira and pr for taking the pairs in a list of files.  https://issues.apache.org/jira/browse/HBASE-26785 but later decided to include here.


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] clarax commented on pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
clarax commented on pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#issuecomment-1057533157


   @wchevreuil @ndimiduk all ready. Please review.


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase-operator-tools] ndimiduk commented on a change in pull request #69: HBASE-24587 hbck2 command should accept one or more files containing …

Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #69:
URL: https://github.com/apache/hbase-operator-tools/pull/69#discussion_r443855362



##########
File path: hbase-hbck2/README.md
##########
@@ -137,12 +138,13 @@ Command:
    Returns the pid(s) of the created AssignProcedure(s) or -1 if none.
    If -i or --inputFiles is specified, pass one or more input file names.
    Each file contains encoded region names, one per line. For example:
-     $ HBCK2 assigns -i fileName1 fileName2
+     $ HBCK2 -i assigns fileName1 fileName2

Review comment:
       The command comes after the `-i`? I think this was correct before the change.




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