PDavid commented on code in PR #167:
URL:
https://github.com/apache/hbase-operator-tools/pull/167#discussion_r4196125932
##########
hbase-table-reporter/src/main/java/org/apache/hbase/reporter/TableReporter.java:
##########
@@ -171,6 +171,26 @@ static void processRowResult(Result result, Sketches
sketches) {
sketches.columnCountSketch.update(columnCount);
}
+ /**
+ * Feed <code>results</code> to <code>sketches</code>, stopping after
<code>limit</code> rows when
+ * <code>limit</code> is positive.
+ * @return Count of rows processed.
+ */
+ static long processResults(Iterable<Result> results, Sketches sketches, int
limit) {
Review Comment:
I really like that you extracted this to a separate testable method here. 👍
##########
hbase-table-reporter/src/main/java/org/apache/hbase/reporter/TableReporter.java:
##########
@@ -437,6 +450,10 @@ public static void main(String[] args)
String opt = limitOption.getOpt();
if (commandLine.hasOption(opt)) {
limit = Integer.parseInt(commandLine.getOptionValue(opt));
+ if (limit <= 0) {
+ usage(options, "Bad limit: " + limit + "; limit must be > 0");
+ System.exit(0);
Review Comment:
Minor question:
Shouldn't we exit here with `System.exit(1)` here as zero indicates
successful run?
I see that in al other non-successful cases we also use `System.exit(0)` so
this is pre-existing behavior.
Is it possible / worth to change that? Or we might break users with using
different exit 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
