[ 
https://issues.apache.org/jira/browse/HBASE-15965?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15356556#comment-15356556
 ] 

Junegunn Choi commented on HBASE-15965:
---------------------------------------

bq. We do not print the return value in interactive mode and keep the output 
clean.

Hi [~Appy], can we reconsider this change? In our case, we take advantage of 
the fact that hbase shell is really a JRuby REPL, and use the return values of 
some commands on the shell. For example,

{code}
# Disable tmp_* tables
list.select { |t| t.start_with? 'tmp' }.each { |t| disable t }
{code}

Actually I was thinking of further extending the return values to contain more 
information, e.g. {{list_snapshot}} returning not only names but also creation 
times, etc. It'll make the command extra useful when you want to do some ad-hoc 
operations on the shell. But I do agree that for most commands, the return 
values are irrelevant and makes the output unnecessarily verbose.

How about if we add an extra set of "query" commands (e.g. {{get_tables}} or 
maybe {{tables?}}) that return values regardless of interactivity? If you like 
the idea I can write up a PoC patch for it.

> 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)

Reply via email to