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



##########
File path: hbase-hbck2/src/test/java/org/apache/hbase/TestHBCK2.java
##########
@@ -266,6 +277,29 @@ public void testFormatReportMissingInMetaOneMissing() 
throws IOException {
     assertTrue(result.contains(expectedResult));
   }
 
+  private void unassigns(List<RegionInfo> regions, String[] regionStrsArray) 
throws IOException {

Review comment:
       nice refactor.

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -294,7 +301,29 @@ 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;
+          while ((regionName = bufferedReader.readLine()) != null) {

Review comment:
       doesn't check style complain about this while-loop construct? Oh, I see 
we have no precommit job for this project :(

##########
File path: hbase-hbck2/src/main/java/org/apache/hbase/HBCK2.java
##########
@@ -109,7 +111,10 @@
   private static final String ADD_MISSING_REGIONS_IN_META_FOR_TABLES =
     "addFsRegionsMissingInMeta";
   private static final String REPORT_MISSING_REGIONS_IN_META = 
"reportMissingRegionsInMeta";
+
   static final String EXTRA_REGIONS_IN_META = "extraRegionsInMeta";
+  static final String ASSIGNS = "assigns";

Review comment:
       nit: I prefer constants used in APIs/CLIs to remain private, and the 
tests to duplicate the values. That way, it's easier to notice a breaking 
change when refactoring existing code -- the test that duplicates the value 
(rather than referring to the constant in the source class) will start breaking.

##########
File path: hbase-hbck2/src/test/java/org/apache/hbase/TestHBCK2.java
##########
@@ -127,32 +130,40 @@ 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[] {});
+
       try (ClusterConnection connection = this.hbck2.connect(); Hbck hbck = 
connection.getHbck()) {
-        List<Long> pids = this.hbck2.unassigns(hbck, regionStrsArray);
-        waitOnPids(pids);
-        for (RegionInfo ri : regions) {
-          RegionState rs = 
TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager().
-              getRegionStates().getRegionState(ri.getEncodedName());
-          LOG.info("RS: {}", rs.toString());
-          assertTrue(rs.toString(), rs.isClosed());
-        }
-        pids = this.hbck2.assigns(hbck, regionStrsArray);
+        unassigns(regions, regionStrsArray);
+        List<Long> pids = this.hbck2.assigns(hbck, regionStrsArray);
         waitOnPids(pids);
-        for (RegionInfo ri : regions) {
-          RegionState rs = 
TEST_UTIL.getHBaseCluster().getMaster().getAssignmentManager().
-              getRegionStates().getRegionState(ri.getEncodedName());
-          LOG.info("RS: {}", rs.toString());
-          assertTrue(rs.toString(), rs.isOpened());
-        }
+        validateOpen(regions);
         // What happens if crappy region list passed?
         pids = this.hbck2.assigns(hbck, Arrays.stream(new String[]{"a", "some 
rubbish name"}).
-            collect(Collectors.toList()).toArray(new String[]{}));
+                collect(Collectors.toList()).toArray(new String[]{}));
         for (long pid : pids) {
           
assertEquals(org.apache.hadoop.hbase.procedure2.Procedure.NO_PROC_ID, pid);
         }
+
+        // test input files
+        unassigns(regions, regionStrsArray);
+        String testFile = "inputForAssignsTest";
+        FileOutputStream output = new FileOutputStream(testFile, false);
+        for (String regionStr : regionStrsArray) {
+          output.write((regionStr + System.lineSeparator()).getBytes());
+        }
+        output.close();
+        String result = testRunWithArgs(new String[] {HBCK2.ASSIGNS, "-i", 
testFile});
+        Scanner scanner = new Scanner(result).useDelimiter("[\\D]+");
+        pids = new ArrayList<>();
+        while (scanner.hasNext()) {
+            pids.add(scanner.nextLong());
+        }
+        scanner.close();
+        //pids = this.hbck2.assigns(hbck, new String[] {"-i", testFile});

Review comment:
       delete stray comment?




----------------------------------------------------------------
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:
[email protected]


Reply via email to