ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nate Cole" <nc...@hortonworks.com>
Subject Re: Review Request 33340: Add Prerequisite Check for Merged Configs
Date Sun, 19 Apr 2015 20:48:56 GMT


> On April 19, 2015, 3:13 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/ConfigMergeHelper.java,
line 156
> > <https://reviews.apache.org/r/33340/diff/1/?file=934992#file934992line156>
> >
> >     I sure hope we always use the same case for "g" or "m".

Good point.  I'll add the caps to the regex too.


> On April 19, 2015, 3:13 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/ConfigMergeHelper.java,
line 60
> > <https://reviews.apache.org/r/33340/diff/1/?file=934992#file934992line60>
> >
> >     I want to confirm my understanding:
> >     If a service is added in the newer stack, then no conflicts.
> >     It's only when the property changes value, or the property is removed entirely
(perhaps component could be removed) that a conflict may arise.

We can never have a service added the newer stack to be in conflict since it couldn't have
been installed for the old stack.


> On April 19, 2015, 3:13 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/checks/HostsRepositoryVersionCheck.java,
line 73
> > <https://reviews.apache.org/r/33340/diff/1/?file=934988#file934988line73>
> >
> >     Why not just do the break inside here?

Result of a million iterations to check it :)  Will fix.


> On April 19, 2015, 3:13 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/checks/ConfigurationMergeCheck.java,
line 60
> > <https://reviews.apache.org/r/33340/diff/1/?file=934987#file934987line60>
> >
> >     findByVersion can return a null, need to check for it.

Will add the null check to the isApplicable(), because if it passes there then it will be
guaranteed to be non-null during perform()


> On April 19, 2015, 3:13 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/checks/ConfigurationMergeCheck.java,
line 98
> > <https://reviews.apache.org/r/33340/diff/1/?file=934987#file934987line98>
> >
> >     My need to check that twv.oldStackValue is not null in the last condition.
> >     
> >     If a property is being added, it will be null in oldStackValue, and say 1 in
both twv.savedValue and twv.newStackValue, and this will generate a conflict even though a
new property is being added.
> >     
> >     Is this the case? If not, can drop the issue.

Good catch, will add a check and a test for that scenario.


- Nate


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


On April 19, 2015, 10:28 a.m., Nate Cole wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33340/
> -----------------------------------------------------------
> 
> (Updated April 19, 2015, 10:28 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
> 
> 
> Bugs: AMBARI-10586
>     https://issues.apache.org/jira/browse/AMBARI-10586
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> * Added prereq check for merged configs
> * Fixed host check to handle when a version is specified, to check the proper version
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/checks/CheckDescription.java 0db235f

>   ambari-server/src/main/java/org/apache/ambari/server/checks/ConfigurationMergeCheck.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/checks/HostsRepositoryVersionCheck.java
0db7e2e 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PreUpgradeCheckResourceProvider.java
ee8cefce 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/dao/RepositoryVersionDAO.java
db5e956 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/RepositoryVersionEntity.java
dd5ac0a 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ConfigMergeHelper.java PRE-CREATION

>   ambari-server/src/main/java/org/apache/ambari/server/state/stack/PrereqCheckStatus.java
3384b85 
>   ambari-server/src/test/java/org/apache/ambari/server/checks/ConfigurationMergeCheckTest.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/checks/HostsRepositoryVersionCheckTest.java
96151af 
> 
> Diff: https://reviews.apache.org/r/33340/diff/
> 
> 
> Testing
> -------
> 
> Manual cluster testing + Automated:
> 
> Tests run: 2867, Failures: 0, Errors: 0, Skipped: 17
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 22:56.903s
> [INFO] Finished at: Sun Apr 19 09:40:46 EDT 2015
> [INFO] Final Memory: 40M/780M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Nate Cole
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message