-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28472/#review64072
-----------------------------------------------------------

Ship it!


The patch looks good, thanks for taking the time to incorporate the requested 
changes.
Please fix the 2 new issues and then feel free to merge the patch.


ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
<https://reviews.apache.org/r/28472/#comment106497>

    There is no matching Logger method signature for error(String format, 
Object arg, Throwable t).  Instead it is matching error(String format, 
Object... arguments) which you don't want because you only have one arg and an 
exception.  The method signature that takes an exception is error(String msg, 
Throwable t) so you could do something like the following:  
LOG.error(String.format("Can not read role command order info %s", 
rcoFilePath), e)



ambari-server/src/main/java/org/apache/ambari/server/state/stack/StackRoleCommandOrder.java
<https://reviews.apache.org/r/28472/#comment106492>

    Just noticed that you are using a deprecated class.  Please replace 
MultiHashMap with the improved MultiValueMap.


- John Speidel


On Dec. 5, 2014, 5:55 p.m., Andrew Onischuk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28472/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2014, 5:55 p.m.)
> 
> 
> Review request for Ambari, Dmitro Lisnichenko and John Speidel.
> 
> 
> Bugs: AMBARI-4504
>     https://issues.apache.org/jira/browse/AMBARI-4504
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> role_command_order is not inherited from the parent stack
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/metadata/RoleCommandOrder.java
>  32668fa 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  2a30e40 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 3629b36 
>   ambari-server/src/main/java/org/apache/ambari/server/state/StackInfo.java 
> f19cf81 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/StackRoleCommandOrder.java
>  PRE-CREATION 
>   ambari-server/src/main/resources/stacks/HDP/2.1/role_command_order.json 
> 2d152ba 
>   ambari-server/src/main/resources/stacks/HDP/2.2/role_command_order.json 
> 72b49fa 
>   
> ambari-server/src/test/java/org/apache/ambari/server/stack/StackManagerTest.java
>  940c65a 
>   ambari-server/src/test/resources/stacks/HDP/2.0.8/role_command_order.json 
> 1404ef6 
>   ambari-server/src/test/resources/stacks/HDP/2.1.1/role_command_order.json 
> 1404ef6 
> 
> Diff: https://reviews.apache.org/r/28472/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Andrew Onischuk
> 
>

Reply via email to