[
https://issues.apache.org/jira/browse/HBASE-15965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15358223#comment-15358223
]
Junegunn Choi commented on HBASE-15965:
---------------------------------------
[~Appy]
I was reminded of the same xkcd strip when I first ran into this :)
bq. You should be able to get the return value using -n (non-interactive) flag.
Yes, but it's quite useful to be able to *incrementally explore* the data
interactively on the REPL. Internally, we've been using a customized version of
the shell with a few extra commands that allow us to explore the status of the
cluster without having to write verbose Java code with Admin API. One such
command is {{regions}}, and this is an example of what we do when things happen.
{code}
# See the number of regions
regions.count
# See the number of regions whose localities are below 50%
regions.select { |r| r[:locality] < 50 }
# Pretty-print those regions grouped by their tables
require 'pp'
pp regions.select { |r| r[:locality] < 50 }.group_by { |r| r[:table] }
# Flush and major compact regions that match the criteria
regions.select { |r| r[:locality] < 50 && r[:files] > 2 }.each { |r| flush
r[:name]; major_compact r[:name] }
{code}
If I had to use {{-n}} flag, I'll have to re-run hbase shell multiple times.
There's nothing wrong with harnessing the power of REPL.
bq. Changing return value to contain more information will break someone else's
workflow
Yes, the concern led me to the idea of introducing new commands instead of
changing the return values of the existing ones.
bq. That approach requires duplicating classes and we over 100 commands
I think we have a misunderstanding here. I was not suggesting that we should
replicate every one of the commands, but to add just a handful of new ones
(tables, snapshots, regions, servers, etc.) under a new command group named
something like QUERY or INSPECTION whose members are supposed to return values
with clear semantics. They aren't for side-effects, they are not responsible
for formatting and printing the output, they should just return values that can
be used by the user programmatically even in "interactive" mode. I don't
disagree with suppressing the return values of the commands in other groups for
cleaner output, but the commands in this group are solely for their return
values, so we make an exception. What do you think?
> Shell test changes. Use @shell.command instead directly calling functions in
> admin.rb and other libraries.
> ----------------------------------------------------------------------------------------------------------
>
> Key: HBASE-15965
> URL: https://issues.apache.org/jira/browse/HBASE-15965
> Project: HBase
> Issue Type: Bug
> Reporter: Appy
> Assignee: Appy
> Fix For: 2.0.0
>
> Attachments: HBASE-15965.master.001.patch,
> HBASE-15965.master.002.patch, HBASE-15965.master.003.patch
>
>
> Testing by executing a command will cover the exact path users will trigger,
> so its better then directly calling library functions in tests. Changing the
> tests to use @shell.command(:<command>, args) to execute them like it's a
> command coming from shell.
> Norm change:
> Commands should print the output user would like to see, but in the end,
> should also return the relevant value. This way:
> - Tests can use returned value to check that functionality works
> - Tests can capture stdout to assert particular kind of output user should
> see.
> - We do not print the return value in interactive mode and keep the output
> clean. See Shell.command() function.
> Bugs found due to this change:
> - Uncovered bug in major_compact.rb with this approach. It was calling
> admin.majorCompact() which doesn't exist but our tests didn't catch it since
> they directly tested admin.major_compact()
> - Enabled TestReplicationShell. If it's bad, flaky infra will take care of it.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
