Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-12 Thread Amruta Borkar 

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

Review request for Ambari, Di Li, Juanjo  Marron, Laszlo Puskas, 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/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/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

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-13 Thread Di Li 

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




ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java
 (line 103)


do you support other type of checks ? such as the property has to be a 
certain value ?



ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/metainfo.xml 
(line 55)


this is already a subsection of "dependency", it can just be called 
"conditions"



ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/metainfo.xml 
(line 56)


same here, no need to reiterate it's for "dependency"



ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/metainfo.xml 
(line 58)


what does "dfs.nameservices" mean in this conditional 
check? as a negative test, what if a user manually added the property to 
hdfs-site.xml ?


- Di Li


On Sept. 12, 2016, 5:45 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 12, 2016, 5:45 p.m.)
> 
> 
> Review request for Ambari, Di Li, Juanjo  Marron, Laszlo Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-13 Thread Di Li 

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



Please add Shantanu as a reviewer

- Di Li


On Sept. 12, 2016, 5:45 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 12, 2016, 5:45 p.m.)
> 
> 
> Review request for Ambari, Di Li, Juanjo  Marron, Laszlo Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-13 Thread Amruta Borkar 

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

(Updated Sept. 13, 2016, 9:28 p.m.)


Review request for Ambari, Shantanu Mundkur, Di Li, Juanjo  Marron, Laszlo 
Puskas, 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 (updated)
-

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

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-13 Thread Amruta Borkar 


> On Sept. 13, 2016, 2:07 p.m., Di Li wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java,
> >  line 103
> > 
> >
> > do you support other type of checks ? such as the property has to be a 
> > certain value ?

Hi Di,
Yes, property value check is handled by the tag  
 as given in the document. This tag is optional and if it is 
present, then the blueprint will be validated for the the  
mentioned in metainfo.xml file. 
This check is performed by following code in isResolved() method:
//if 'propertyValue' is null then it is assumed that condition only 
checks if the 'property' exists or not
if(propertyValue == null || 
propertyValue.equals(properties.get(configType).get(property))) return true;

For this scenario (NAMENODE HA) we do not have conditional dependency based on 
propertyValue hence I have not added it in the metainfo.xml file.


> On Sept. 13, 2016, 2:07 p.m., Di Li wrote:
> > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/metainfo.xml,
> >  line 58
> > 
> >
> > what does "dfs.nameservices" mean in this 
> > conditional check? as a negative test, what if a user manually added the 
> > property to hdfs-site.xml ?

When a strucutre like  
   .. 
   ... 
   
is present in metainfo.xml file code will look to see if this property is 
present in blueprint. If not then the  will not be considered 
mandatory. 

I have modified the patch to consider configurations provided in hdfs-site.xml.


> On Sept. 13, 2016, 2:07 p.m., Di Li wrote:
> > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/metainfo.xml,
> >  line 55
> > 
> >
> > this is already a subsection of "dependency", it can just be called 
> > "conditions"

Modified the patch.


> On Sept. 13, 2016, 2:07 p.m., Di Li wrote:
> > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/metainfo.xml,
> >  line 56
> > 
> >
> > same here, no need to reiterate it's for "dependency"

Modified the patch.


- Amruta


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


On Sept. 13, 2016, 9:28 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 13, 2016, 9:28 p.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Juanjo  Marron, Laszlo 
> Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-14 Thread Di Li 

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




ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java
 (line 35)


should configType, property, value be mapped to xml elements explicitly ?


- Di Li


On Sept. 13, 2016, 9:28 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 13, 2016, 9:28 p.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Juanjo  Marron, Laszlo 
> Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-14 Thread Amruta Borkar 


> On Sept. 14, 2016, 5:11 p.m., Di Li wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java,
> >  line 35
> > 
> >
> > should configType, property, value be mapped to xml elements explicitly 
> > ?

Hi Di,
In this case mapping should not be necessary as properties in class have same 
names as defined in metainfo.xml and DependencyConditions class is mapped to 
xml element 'condition' in DependencyInfo class.


- Amruta


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


On Sept. 13, 2016, 9:28 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 13, 2016, 9:28 p.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Juanjo  Marron, Laszlo 
> Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-16 Thread Robert Nettleton 

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



Thanks for providing this patch.  This is a feature in Blueprints that is 
sorely needed, and will be beneficial for Blueprints generally. 

I would ask, if possible, that some consideration be put into making the XML 
syntax for specifying conditions on dependencies a little more generic, and 
then making the two conditional types implemented here be subtypes of the 
generic type.  My thinking here is that having this be just a little more 
flexible may benefit Blueprints in the future, as more conditional types may be 
required for different usage types.  

I agree completely that conditions based on configuration are the most common, 
and the two conditional types implemented are likely to be used in a variety of 
ways across the stacks, but it would be great if these were treated as specific 
sub-types of the condition, to make future conditions easier to add. 

Can you please add Jayush Lunia to this review list?  I believe he's looking at 
some stack-level refactorings, so it would be good to get his input as well.

Thanks.


ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java
 (line 37)


I think this is a great idea, but I'd recommend a slightly more generic 
approach.

Adding conditional dependencies is a great idea, and I would agree that by 
far that the most common case is configuration types. 

That being said, it might be a good idea to consider making this a little 
more generic, and then having the 'config-type-exists' and 
'property-value-equals' conditions be specific cases of that generic type. 

It might be useful to add a new 'condition-type' field to 
DependencyConditionInfo, so that the stacks indicate the type of condition.  
This will make it easier to visually parse the stacks in the future, and allows 
for more conditional types to be added.  

Having this be a little more generic would allow future conditional types 
to be added without too much effort.  Off the top of my head, I can't think of 
any, but I would expect that future use cases might open up possibilities of 
declaring conditions in a variety of ways, to simplify stack development and 
maintenance.



ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintValidatorImplTest.java
 (line 87)


Minor issue:

I couldn't find any usages of this field.  Is this used in the unit tests?



ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintValidatorImplTest.java
 (line 357)


Minor issue:

Could you maybe rename this method, so that the intent of the test is a 
little clearer?  

Maybe something to indicate that the conditionally dependent component is 
missing.

Sorry to nit-pick here, but that might make it easier to maintain this test 
over time. 

Thanks.


- Robert Nettleton


On Sept. 13, 2016, 9:28 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 13, 2016, 9:28 p.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Juanjo  Marron, Laszlo 
> Puskas, 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/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/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.
> 
> 
> T

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-20 Thread Jayush Luniya 


> On Sept. 16, 2016, 3:44 p.m., Robert Nettleton wrote:
> > Thanks for providing this patch.  This is a feature in Blueprints that is 
> > sorely needed, and will be beneficial for Blueprints generally. 
> > 
> > I would ask, if possible, that some consideration be put into making the 
> > XML syntax for specifying conditions on dependencies a little more generic, 
> > and then making the two conditional types implemented here be subtypes of 
> > the generic type.  My thinking here is that having this be just a little 
> > more flexible may benefit Blueprints in the future, as more conditional 
> > types may be required for different usage types.  
> > 
> > I agree completely that conditions based on configuration are the most 
> > common, and the two conditional types implemented are likely to be used in 
> > a variety of ways across the stacks, but it would be great if these were 
> > treated as specific sub-types of the condition, to make future conditions 
> > easier to add. 
> > 
> > Can you please add Jayush Lunia to this review list?  I believe he's 
> > looking at some stack-level refactorings, so it would be good to get his 
> > input as well.
> > 
> > Thanks.

Generally I agree with the comments here. We should categorize the type of 
condition too by adding condition-type property (i.e. 
config).


- Jayush


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


On Sept. 16, 2016, 4:57 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 16, 2016, 4:57 p.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
> Marron, Laszlo Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-20 Thread Amruta Borkar 

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

(Updated Sept. 20, 2016, 11:18 p.m.)


Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
Marron, Laszlo Puskas, and Robert Nettleton.


Changes
---

Hello,  I have added  tag to represent different 
condition-types and made two sub-types 'IF-PROPERTY-EXISTS' and 
'PROPERTY-VALUE-EQUALS'. Also addressed other two changes suggested by Robert. 
I am still working on adding more test cases. Please review and provide 
feedback.


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

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

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-28 Thread Robert Nettleton 

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


Fix it, then Ship it!




Overall, the patch looks fine to me.  I've just added a few minor issues that 
might be useful to consider.  

Thanks again for providing this patch!


ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java
 (line 51)


Minor issue:

Since the PropertyValueEquals condition also requires that the property 
exists, perhaps this implementation should inherit from 
PropertyExistsDependencyCondition? 

This might be worth exploring, just from a code maintenance perspective.



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java
 (line 53)


Minor issue: 

It might be a good idea to make these fields final, since they seem to be 
immutable fields in these classes.



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java
 (line 57)


Minor Suggestion(and I won't add an issue here):

If assertions were added to the constructor to verify that the parameters 
are not null, then the logic in the isResolved() methods could be simplified a 
bit.


- Robert Nettleton


On Sept. 20, 2016, 11:18 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 20, 2016, 11:18 p.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
> Marron, Laszlo Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-29 Thread Amruta Borkar 

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

(Updated Sept. 30, 2016, 12:39 a.m.)


Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
Marron, Laszlo Puskas, 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 (updated)
-

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

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-09-29 Thread Alejandro Fernandez 

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


Ship it!




Ship It!

- Alejandro Fernandez


On Sept. 30, 2016, 12:39 a.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 30, 2016, 12:39 a.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
> Marron, Laszlo Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-10-03 Thread Di Li 

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


Ship it!




Ship It!

- Di Li


On Sept. 30, 2016, 12:39 a.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 30, 2016, 12:39 a.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
> Marron, Laszlo Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-10-05 Thread Jayush Luniya 

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


Ship it!




Ship It!

- Jayush Luniya


On Sept. 30, 2016, 12:39 a.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 30, 2016, 12:39 a.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
> Marron, Laszlo Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-10-07 Thread Nate Cole 

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



This code is breaking tests, and has been reverted from trunk.  Please fix the 
tests, and also fix the formatting.  Ambari is standardized on two space tabs.  
DependencyConditionInfo is a undocumented and unformatted class.  In addition, 
you shouldn't be making subclasses as internal classes of the parent abstract 
class.

Di Li, when you commit code on behalf of someone else, you must make the 
message like so: "AMBARI-X. Some message (Joe Smith via dili)"

- Nate Cole


On Sept. 29, 2016, 8:39 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 29, 2016, 8:39 p.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
> Marron, Laszlo Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-10-07 Thread Di Li 


> On Oct. 7, 2016, 7:39 p.m., Nate Cole wrote:
> > This code is breaking tests, and has been reverted from trunk.  Please fix 
> > the tests, and also fix the formatting.  Ambari is standardized on two 
> > space tabs.  DependencyConditionInfo is a undocumented and unformatted 
> > class.  In addition, you shouldn't be making subclasses as internal classes 
> > of the parent abstract class.
> > 
> > Di Li, when you commit code on behalf of someone else, you must make the 
> > message like so: "AMBARI-X. Some message (Joe Smith via dili)"

Anything else you think I should put in as the comment, Nate? I did put (Amruta 
Borkar via dili) at the end of the comment though...

commit f6124a056d2a8ed16bec917775b9d3554ab5d74d
Author: Di Li 
Date:   Fri Oct 7 13:34:54 2016 -0400

AMBARI-18355: Introduce conditional dependencies in stack defition to 
handle blueprint validation gracefully (Amruta Borkar via dili)


- Di


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


On Sept. 30, 2016, 12:39 a.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Sept. 30, 2016, 12:39 a.m.)
> 
> 
> Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
> Marron, Laszlo Puskas, 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/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/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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-10-12 Thread Amruta Borkar 

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

(Updated Oct. 12, 2016, 11:42 p.m.)


Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
Marron, Laszlo Puskas, Nate Cole, and Robert Nettleton.


Changes
---

Hello Nate, I have modified the patch according to your comments. Could you 
please take a look? Thank you


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

  
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

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-10-13 Thread Nate Cole 


> On Oct. 7, 2016, 3:39 p.m., Nate Cole wrote:
> > This code is breaking tests, and has been reverted from trunk.  Please fix 
> > the tests, and also fix the formatting.  Ambari is standardized on two 
> > space tabs.  DependencyConditionInfo is a undocumented and unformatted 
> > class.  In addition, you shouldn't be making subclasses as internal classes 
> > of the parent abstract class.
> > 
> > Di Li, when you commit code on behalf of someone else, you must make the 
> > message like so: "AMBARI-X. Some message (Joe Smith via dili)"
> 
> Di Li wrote:
> Anything else you think I should put in as the comment, Nate? I did put 
> (Amruta Borkar via dili) at the end of the comment though...
> 
> commit f6124a056d2a8ed16bec917775b9d3554ab5d74d
> Author: Di Li 
> Date:   Fri Oct 7 13:34:54 2016 -0400
> 
> AMBARI-18355: Introduce conditional dependencies in stack defition to 
> handle blueprint validation gracefully (Amruta Borkar via dili)

No no, that was my mistake - my browser had cut off the comment so all I saw 
the author's name.


- Nate


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


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

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-10-13 Thread Nate Cole 

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


indent here should be two spaces



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionAdapter.java
 (lines 51 - 53)


two spaces



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionAdapter.java
 (lines 59 - 66)


two spaces



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionAdapter.java
 (lines 64 - 70)


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)


two spaces



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyConditionInfo.java
 (lines 73 - 75)


two spaces



ambari-server/src/main/java/org/apache/ambari/server/state/DependencyInfo.java 
(lines 169 - 171)


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)


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)


This is a bit of an odd syntax.  the  element should be a 
different class then you don't need the  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:


  

  
  

  




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

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-11-01 Thread Amruta Borkar 

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

(Updated Nov. 1, 2016, 6:57 p.m.)


Review request for Ambari, Shantanu Mundkur, Di Li, Jayush Luniya, Juanjo  
Marron, Laszlo Puskas, Nate Cole, and Robert Nettleton.


Changes
---

Hello Nate, Could you please review the updated patch. Thank you


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

  
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 
9c1387d 
  
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

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-11-08 Thread Amruta Borkar 


> On Oct. 13, 2016, 1:24 p.m., Nate Cole wrote:
> > ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/metainfo.xml,
> >  lines 43-47
> > 
> >
> > This is a bit of an odd syntax.  the  element should be a 
> > different class then you don't need the  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:
> > 
> > 
> > 
> >   
> > 
> >   
> >   
> > 
> >   
> > 
> > 

Hello Nate,
Could you please provide your feedback on the updated patch?

Thank you,
Amruta


- Amruta


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


On Nov. 1, 2016, 6:57 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Nov. 1, 2016, 6:57 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/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 
> 9c1387d 
>   
> 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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-11-10 Thread Nate Cole 

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


Ship it!




Ship It!

- Nate Cole


On Nov. 1, 2016, 2:57 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Nov. 1, 2016, 2:57 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/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 
> 9c1387d 
>   
> 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
> 
>

Re: Review Request 51815: Introduce conditional dependencies in stack defition to handle blueprint validation gracefully

2016-11-10 Thread Amruta Borkar 


> On Nov. 10, 2016, 4:22 p.m., Nate Cole wrote:
> > Ship It!

Thank you Nate, could you please help me push the patch to trunk? I do not have 
that access.


- Amruta


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


On Nov. 1, 2016, 6:57 p.m., Amruta Borkar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51815/
> ---
> 
> (Updated Nov. 1, 2016, 6:57 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/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 
> 9c1387d 
>   
> 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
> 
>