> On March 21, 2016, 11:33 a.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java,
> >  lines 1437-1438
> > <https://reviews.apache.org/r/45101/diff/1/?file=1308341#file1308341line1437>
> >
> >     I would have thought that the fix would have been somewhere within the 
> > blueprint processing code rather than here since it attempts to set the 
> > `cluster-env` config multiple times with the same tag.
> >     
> >     If you skip the setting of this, wont you loose data?
> 
> Myroslav Papirkovskyy wrote:
>     Actually no. We skip creating config if config with such type already 
> mapped to cluster and it has exactly same properties and their values.
>     This section of code forces config creation in some edge cases related to 
> service removal.
>     It is ok to skip this for all config types not related to services.

My concern is that cluster-env may be updated in one of those calls to set it.  
We don't want to loose that change or run into the issue that lead to this fix. 

However, maybe we _know_ that there is no change and thus my point is 
irrelevant.


- Robert


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


On March 21, 2016, 10:23 a.m., Sebastian Toader wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45101/
> -----------------------------------------------------------
> 
> (Updated March 21, 2016, 10:23 a.m.)
> 
> 
> Review request for Ambari, Myroslav Papirkovskyy, Robert Levas, and Sandor 
> Magyari.
> 
> 
> Bugs: AMBARI-15489
>     https://issues.apache.org/jira/browse/AMBARI-15489
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The logic that determines if a new version has to be created for a 
> configuration should skip cluster wide config types when verifying current 
> desired config against service configs as these are not service configs.
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  ac2fb22 
> 
> Diff: https://reviews.apache.org/r/45101/diff/
> 
> 
> Testing
> -------
> 
> Manual testing done.
> 
> Unit tests results:
> Results :
> 
> Tests run: 3534, Failures: 0, Errors: 0, Skipped: 36
> 
> 
> Thanks,
> 
> Sebastian Toader
> 
>

Reply via email to