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/09 23:01:04 UTC
[GitHub] [hbase-operator-tools] clarax opened a new pull request #64: HBASE-23927 recommit the fix with updates for PR review feedback
clarax opened a new pull request #64:
URL: https://github.com/apache/hbase-operator-tools/pull/64
----------------------------------------------------------------
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] huaxiangsun merged pull request #64: HBASE-23927 recommit the fix with updates for PR review feedback
Posted by GitBox <gi...@apache.org>.
huaxiangsun merged pull request #64:
URL: https://github.com/apache/hbase-operator-tools/pull/64
----------------------------------------------------------------
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] ndimiduk commented on a change in pull request #64: HBASE-23927 recommit the fix with updates for PR review feedback
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org