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




ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionAdapter.java
 (lines 27 - 29)
<https://reviews.apache.org/r/51815/#comment221495>

    indent here should be two spaces



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionAdapter.java
 (lines 51 - 53)
<https://reviews.apache.org/r/51815/#comment221496>

    two spaces



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionAdapter.java
 (lines 59 - 66)
<https://reviews.apache.org/r/51815/#comment221497>

    two spaces



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionAdapter.java
 (lines 64 - 70)
<https://reviews.apache.org/r/51815/#comment221504>

    This is odd - it's like you're forcing an additional element to determine a 
class name where java XML parsing gives that to you for free.  See comment 
regarding schema.



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java
 (lines 49 - 52)
<https://reviews.apache.org/r/51815/#comment221498>

    two spaces



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java
 (lines 73 - 75)
<https://reviews.apache.org/r/51815/#comment221499>

    two spaces



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyInfo.java 
(lines 169 - 171)
<https://reviews.apache.org/r/51815/#comment221500>

    We're not above using Apache commons:  
!CollectionUtils.isEmpty(dependencyConditions)



ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintValidatorImpl.java
 (lines 294 - 296)
<https://reviews.apache.org/r/51815/#comment221501>

    this looks like it will continue to check even if a prior dependency is not 
resolved.  Do you need to continue checking or can you 'break' after 
conditionsSatisfied is set to false?



ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/metainfo.xml 
(lines 43 - 47)
<https://reviews.apache.org/r/51815/#comment221503>

    This is a bit of an odd syntax.  the <condition> element should be a 
different class then you don't need the <condition-type> element.  Now, these 
metainfo.xml files don't have schema for validation, but java XML is able to 
use correct classes based on the element name.  So for example, you would have 
two types such that:
    
    <conditions>
      <condition xsi:type="property-exists">
        <elements for property-exists>
      </condition>
      <condition xsi:type="property-equals">
        <elements for property-equals>
      </condition>
    
    </conditions>


- Nate Cole


On Oct. 12, 2016, 7:42 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2016, 7:42 p.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
> Marron, Laszlo Puskas, Nate Cole, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-18355
>     https://issues.apache.org/jira/browse/AMBARI-18355
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Currently stack definitions do not list conditional dependencies, adding 
> those to the stack definitions would make it easy to validate errors in case 
> of blueprint deployment. Please refer to document attached to Jira
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionAdapter.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/DependencyInfo.java
>  e3db662 
>   
> ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintValidatorImpl.java
>  a5f33ff 
>   
> ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/metainfo.xml 
> 65d166a 
>   
> ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintImplTest.java
>  ff9af17 
>   
> ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintValidatorImplTest.java
>  b1de8ef 
> 
> Diff: https://reviews.apache.org/r/51815/diff/
> 
> 
> Testing
> -------
> 
> Written Junit test cases. Perfomred manual testing to check the Namenode HA 
> component dependency. Was able to proceed with the installation for valid 
> blueprint. and got validation error message while registering blueprint when 
> the Blueprint did not satisfy the conditional dependencies.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>

Reply via email to