ndimiduk commented on a change in pull request #64:
URL: 
https://github.com/apache/hbase-operator-tools/pull/64#discussion_r437772726



##########
File path: hbase-hbck2/src/test/java/org/apache/hbase/TestHBCK2.java
##########
@@ -70,6 +73,8 @@
   private static final TableName TABLE_NAME = 
TableName.valueOf(TestHBCK2.class.getSimpleName());
   private static final TableName REGION_STATES_TABLE_NAME = TableName.
     valueOf(TestHBCK2.class.getSimpleName() + "-REGIONS_STATES");
+  private final static String ASSIGNS = "assigns";

Review comment:
       thanks!

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -294,7 +300,30 @@ int setRegionState(ClusterConnection connection, String 
region,
       return null;
     }
     boolean overrideFlag = commandLine.hasOption(override.getOpt());
-    return hbck.assigns(commandLine.getArgList(), overrideFlag);
+
+    List<String> argList = commandLine.getArgList();
+    if (!commandLine.hasOption(inputFile.getOpt())) {
+      return hbck.assigns(argList, overrideFlag);
+    } else {

Review comment:
       nit: else block is redundant to the early `return` the line prior.

##########
File path: hbase-hbck2/src/test/java/org/apache/hbase/TestHBCK2.java
##########
@@ -127,32 +132,39 @@ public void testAssigns() throws IOException {
             getRegionStates().getRegionState(ri.getEncodedName());
         LOG.info("RS: {}", rs.toString());
       }
-      List<String> regionStrs =
-          
regions.stream().map(RegionInfo::getEncodedName).collect(Collectors.toList());
-      String [] regionStrsArray = regionStrs.toArray(new String[] {});
+      String [] regionStrsArray  =
+          
regions.stream().map(RegionInfo::getEncodedName).collect(Collectors.toList())
+                  .toArray(new String[] {});

Review comment:
       You can skip the `collect` and go directly 
[`toArray`](https://docs.oracle.com/javase/8/docs/api/java/util/stream/Stream.html#toArray-java.util.function.IntFunction-).

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -294,7 +300,30 @@ int setRegionState(ClusterConnection connection, String 
region,
       return null;
     }
     boolean overrideFlag = commandLine.hasOption(override.getOpt());
-    return hbck.assigns(commandLine.getArgList(), overrideFlag);
+
+    List<String> argList = commandLine.getArgList();
+    if (!commandLine.hasOption(inputFile.getOpt())) {
+      return hbck.assigns(argList, overrideFlag);
+    } else {
+      List<String> assignmentList = new ArrayList<>();
+      for (String filePath : argList) {
+        try {
+          File file = new File(filePath);
+          FileReader fileReader = new FileReader(file);
+          BufferedReader bufferedReader = new BufferedReader(fileReader);
+          String regionName = bufferedReader.readLine().trim();
+          while (regionName != null) {

Review comment:
       Sorry I wasn't more specific before. How about something like 
[IOUtils.lineIterator](http://commons.apache.org/proper/commons-io/javadocs/api-release/org/apache/commons/io/IOUtils.html#lineIterator-java.io.InputStream-java.nio.charset.Charset-)?
 That way there's no infinite loops and no null checking, just a simple for 
loop with an iterator.

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -441,16 +470,20 @@ private static void 
usageAddFsRegionsMissingInMeta(PrintWriter writer) {
   }
 
   private static void usageAssigns(PrintWriter writer) {
-    writer.println(" " + ASSIGNS + " [OPTIONS] <ENCODED_REGIONNAME>...");
+    writer.println(" " + ASSIGNS + " [OPTIONS] 
<ENCODED_REGIONNAME/INPUTFILES_FOR_REGIONNAMES>...");
     writer.println("   Options:");
     writer.println("    -o,--override  override ownership by another 
procedure");
+    writer.println("    -i,--inputFiles  take one or more files of encoded 
region names");

Review comment:
       yikes! i'm surprised our cli parsing library doesn't handle generating a 
help message from arguments on our behalf :(




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to