Re: Review Request 48403: Fixed implementation of on-ambari-upgrade support. Patch 1 - change validation rules and available fields

2016-06-09 Thread Sumit Mohanty 


> On June 8, 2016, 7:21 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java,
> >  line 98
> > 
> >
> > So I had thought that we decided to make these optional (which you did) 
> > but that if you didn't specify them at all, nothing would happen - they'd 
> > all be false. This would prevent localhost from being accidentally added.
> > 
> > Am I wrong? Seems like it's a good balance - we'll allow XML files with 
> > no upgrade behavior elements, but if you don't specify them, then they are 
> > all false. This would prevent the unintentional adding of properties (like 
> > localhost fields or fields which need to be calculated)
> 
> Dmitro Lisnichenko wrote:
> I think making this field false by default would not make a trick because 
> Sumit wants existing customer stacks (that have no on-ambari-upgrade tags) to 
> work as before patch. That's why we would have to make false a default

My major concern was that so far the default behavior has been "add on 
ambari-upgrade". So I was not sure if we can modify the default behaviour 
without any side effect.
>> That's why we would have to make false a default
Dmitro I assume  you mean "add=true" as default?


- Sumit


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


On June 9, 2016, 1:12 p.m., Dmitro Lisnichenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48403/
> ---
> 
> (Updated June 9, 2016, 1:12 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Andrew Onischuk, Jonathan 
> Hurley, Jayush Luniya, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-17112
> https://issues.apache.org/jira/browse/AMBARI-17112
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We decided to split changes to stack config xmls and processing of them into 
> few patches.
> Current patch changes validation rules and available fields
> 
> Splitted PropertyUpgradeBehavior into 2 classes.
> PropertyAmbariUpgradeBehavior - defines behavior for ambari upgrade.
> on-ambari-upgrade now has the same 3 attributes, but all are optional. 
> Default value for add is true, false for others.
> 
> PropertyStackUpgradeBehavior - defines behavior for stack upgrade.
> on-stack-upgrade now has one attribute - add, and it is required.
> 
> Modified property update script and xsd schema to satisfy new requirements.
> 
> If patch looks good, I'll modify all stack configs to comply with it and 
> commit both patches at the same time.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java 
> fba2daa 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyUpgradeBehavior.java
>  de2e342 
>   
> ambari-server/src/main/resources/scripts/configurations-set-default-update-policy.sh
>  930f778 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/PropertyInfoTest.java
>  9a3d195 
> 
> Diff: https://reviews.apache.org/r/48403/diff/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>

Re: Review Request 48403: Fixed implementation of on-ambari-upgrade support. Patch 1 - change validation rules and available fields

2016-06-09 Thread Nate Cole 

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


Ship it!




Ship It!

- Nate Cole


On June 9, 2016, 9:12 a.m., Dmitro Lisnichenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48403/
> ---
> 
> (Updated June 9, 2016, 9:12 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Andrew Onischuk, Jonathan 
> Hurley, Jayush Luniya, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-17112
> https://issues.apache.org/jira/browse/AMBARI-17112
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We decided to split changes to stack config xmls and processing of them into 
> few patches.
> Current patch changes validation rules and available fields
> 
> Splitted PropertyUpgradeBehavior into 2 classes.
> PropertyAmbariUpgradeBehavior - defines behavior for ambari upgrade.
> on-ambari-upgrade now has the same 3 attributes, but all are optional. 
> Default value for add is true, false for others.
> 
> PropertyStackUpgradeBehavior - defines behavior for stack upgrade.
> on-stack-upgrade now has one attribute - add, and it is required.
> 
> Modified property update script and xsd schema to satisfy new requirements.
> 
> If patch looks good, I'll modify all stack configs to comply with it and 
> commit both patches at the same time.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java 
> fba2daa 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyUpgradeBehavior.java
>  de2e342 
>   
> ambari-server/src/main/resources/scripts/configurations-set-default-update-policy.sh
>  930f778 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/PropertyInfoTest.java
>  9a3d195 
> 
> Diff: https://reviews.apache.org/r/48403/diff/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>

Re: Review Request 48403: Fixed implementation of on-ambari-upgrade support. Patch 1 - change validation rules and available fields

2016-06-09 Thread Andrew Onischuk 

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


Ship it!




Ship It!

- Andrew Onischuk


On June 9, 2016, 1:12 p.m., Dmitro Lisnichenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48403/
> ---
> 
> (Updated June 9, 2016, 1:12 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Andrew Onischuk, Jonathan 
> Hurley, Jayush Luniya, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-17112
> https://issues.apache.org/jira/browse/AMBARI-17112
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We decided to split changes to stack config xmls and processing of them into 
> few patches.
> Current patch changes validation rules and available fields
> 
> Splitted PropertyUpgradeBehavior into 2 classes.
> PropertyAmbariUpgradeBehavior - defines behavior for ambari upgrade.
> on-ambari-upgrade now has the same 3 attributes, but all are optional. 
> Default value for add is true, false for others.
> 
> PropertyStackUpgradeBehavior - defines behavior for stack upgrade.
> on-stack-upgrade now has one attribute - add, and it is required.
> 
> Modified property update script and xsd schema to satisfy new requirements.
> 
> If patch looks good, I'll modify all stack configs to comply with it and 
> commit both patches at the same time.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java 
> fba2daa 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyUpgradeBehavior.java
>  de2e342 
>   
> ambari-server/src/main/resources/scripts/configurations-set-default-update-policy.sh
>  930f778 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/PropertyInfoTest.java
>  9a3d195 
> 
> Diff: https://reviews.apache.org/r/48403/diff/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>

Re: Review Request 48403: Fixed implementation of on-ambari-upgrade support. Patch 1 - change validation rules and available fields

2016-06-08 Thread Dmitro Lisnichenko 


> On June 8, 2016, 10:21 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java,
> >  line 98
> > 
> >
> > So I had thought that we decided to make these optional (which you did) 
> > but that if you didn't specify them at all, nothing would happen - they'd 
> > all be false. This would prevent localhost from being accidentally added.
> > 
> > Am I wrong? Seems like it's a good balance - we'll allow XML files with 
> > no upgrade behavior elements, but if you don't specify them, then they are 
> > all false. This would prevent the unintentional adding of properties (like 
> > localhost fields or fields which need to be calculated)

I think making this field false by default would not make a trick because Sumit 
wants existing customer stacks (that have no on-ambari-upgrade tags) to work as 
before patch. That's why we would have to make false a default


- Dmitro


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


On June 8, 2016, 1:06 p.m., Dmitro Lisnichenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48403/
> ---
> 
> (Updated June 8, 2016, 1:06 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-17112
> https://issues.apache.org/jira/browse/AMBARI-17112
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We decided to split changes to stack config xmls and processing of them into 
> few patches.
> Current patch changes validation rules and available fields
> 
> Splitted PropertyUpgradeBehavior into 2 classes.
> PropertyAmbariUpgradeBehavior - defines behavior for ambari upgrade.
> on-ambari-upgrade now has the same 3 attributes, but all are optional. 
> Default value for add is true, false for others.
> 
> PropertyStackUpgradeBehavior - defines behavior for stack upgrade.
> on-stack-upgrade now has one attribute - add, and it is required.
> 
> Modified property update script and xsd schema to satisfy new requirements.
> 
> If patch looks good, I'll modify all stack configs to comply with it and 
> commit both patches at the same time.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java 
> fba2daa 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyUpgradeBehavior.java
>  de2e342 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyUpgradeBehavior.java
>  de2e342 
>   ambari-server/src/main/resources/configuration-schema.xsd d49cbf8 
>   
> ambari-server/src/main/resources/scripts/configurations-set-default-update-policy.sh
>  930f778 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/PropertyInfoTest.java
>  9a3d195 
> 
> Diff: https://reviews.apache.org/r/48403/diff/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>

Re: Review Request 48403: Fixed implementation of on-ambari-upgrade support. Patch 1 - change validation rules and available fields

2016-06-08 Thread Nate Cole 

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




ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java 
(lines 53 - 54)


Is this being removed to focus only on Ambari Upgrade


- Nate Cole


On June 8, 2016, 6:06 a.m., Dmitro Lisnichenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48403/
> ---
> 
> (Updated June 8, 2016, 6:06 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-17112
> https://issues.apache.org/jira/browse/AMBARI-17112
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> We decided to split changes to stack config xmls and processing of them into 
> few patches.
> Current patch changes validation rules and available fields
> 
> Splitted PropertyUpgradeBehavior into 2 classes.
> PropertyAmbariUpgradeBehavior - defines behavior for ambari upgrade.
> on-ambari-upgrade now has the same 3 attributes, but all are optional. 
> Default value for add is true, false for others.
> 
> PropertyStackUpgradeBehavior - defines behavior for stack upgrade.
> on-stack-upgrade now has one attribute - add, and it is required.
> 
> Modified property update script and xsd schema to satisfy new requirements.
> 
> If patch looks good, I'll modify all stack configs to comply with it and 
> commit both patches at the same time.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java 
> fba2daa 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyUpgradeBehavior.java
>  de2e342 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/PropertyUpgradeBehavior.java
>  de2e342 
>   ambari-server/src/main/resources/configuration-schema.xsd d49cbf8 
>   
> ambari-server/src/main/resources/scripts/configurations-set-default-update-policy.sh
>  930f778 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/PropertyInfoTest.java
>  9a3d195 
> 
> Diff: https://reviews.apache.org/r/48403/diff/
> 
> 
> Testing
> ---
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>

Review Request 48403: Fixed implementation of on-ambari-upgrade support. Patch 1 - change validation rules and available fields

2016-06-08 Thread Dmitro Lisnichenko 

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

Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush Luniya, 
Nate Cole, and Sumit Mohanty.


Bugs: AMBARI-17112
https://issues.apache.org/jira/browse/AMBARI-17112


Repository: ambari


Description
---

We decided to split changes to stack config xmls and processing of them into 
few patches.
Current patch changes validation rules and available fields

Splitted PropertyUpgradeBehavior into 2 classes.
PropertyAmbariUpgradeBehavior - defines behavior for ambari upgrade.
on-ambari-upgrade now has the same 3 attributes, but all are optional. Default 
value for add is true, false for others.

PropertyStackUpgradeBehavior - defines behavior for stack upgrade.
on-stack-upgrade now has one attribute - add, and it is required.

Modified property update script and xsd schema to satisfy new requirements.

If patch looks good, I'll modify all stack configs to comply with it and commit 
both patches at the same time.


Diffs
-

  ambari-server/src/main/java/org/apache/ambari/server/state/PropertyInfo.java 
fba2daa 
  
ambari-server/src/main/java/org/apache/ambari/server/state/PropertyUpgradeBehavior.java
 de2e342 
  
ambari-server/src/main/java/org/apache/ambari/server/state/PropertyUpgradeBehavior.java
 de2e342 
  ambari-server/src/main/resources/configuration-schema.xsd d49cbf8 
  
ambari-server/src/main/resources/scripts/configurations-set-default-update-policy.sh
 930f778 
  
ambari-server/src/test/java/org/apache/ambari/server/state/PropertyInfoTest.java
 9a3d195 

Diff: https://reviews.apache.org/r/48403/diff/


Testing
---

mvn clean test


Thanks,

Dmitro Lisnichenko