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

Mark Grover commented on BIGTOP-1125:
-------------------------------------

Thanks Sean! It's looking good!

If you'd allow me to pick out another nit. In the case statement at the very 
bottom, we are still picking up the return code implicitly. What do you think 
about perhaps, explicitly collecting the code returned by each of the functions 
(start, stop, etc.) in a new variable and return that variable in the end 
instead of $?

That might make the code less fragile. Consider the scenario where someone 
comes and adds an echo statement right after the call to restart(), that would 
erroneously change the return code to always be true, no? 

> Return value does not reflect status checks
> -------------------------------------------
>
>                 Key: BIGTOP-1125
>                 URL: https://issues.apache.org/jira/browse/BIGTOP-1125
>             Project: Bigtop
>          Issue Type: Bug
>    Affects Versions: 0.6.0
>            Reporter: Sean Mackrory
>            Assignee: Sean Mackrory
>             Fix For: 0.8.0
>
>         Attachments: 
> 0001-BIGTOP-1125.-Return-value-does-not-reflect-status-ch.patch, 
> 0001-BIGTOP-1125.-Return-value-does-not-reflect-status-ch.patch, 
> bigtop-1125-test.txt
>
>
> The init script for HBase RegionServers that supports multiple processes is 
> always returning 0. It should return 0 on success of non-zero on failure. In 
> the case of running multiple processes, failure is defined as a failure 
> There are already different constants defined in the file (e.g. different 
> values for failures in all process, failures in some processes, failures in 
> no processes... etc.), we just need to return them properly.
> Although this affects the 0.7.0 RC, I don't consider it a big enough deal to 
> warrant -1'ing the RC, which I'm still testing.



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to