> On May 17, 2016, 2:30 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java,
> >  lines 112-113
> > <https://reviews.apache.org/r/45169/diff/2/?file=1382544#file1382544line112>
> >
> >     With no other exceptions, can just use "regular" logging syntax:  
> > LOG.debug("Service upgrades folder {} for service {} in stack {} is 
> > empty.", absUpgradesDir, serviceDir.getName(), stackId)

This code was copied and pasted from the same method which also deals with the 
package dir.


> On May 17, 2016, 2:30 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java,
> >  lines 116-117
> > <https://reviews.apache.org/r/45169/diff/2/?file=1382544#file1382544line116>
> >
> >     Maybe LOG.info() here.  If nothing happens we'll have no record unless 
> > DEBUG is enabled (which is, admittedly, an awful idea)

It is normal for a service to not have an upgrades folder.  The upgrade folder 
is only required IF a service wants to extend the upgrade xml with its own 
specific logic.  There is no reason to litter the logs with this.

This code was copied and pasted from the same method which also deals with the 
package dir.


> On May 17, 2016, 2:30 p.m., Nate Cole wrote:
> > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml, 
> > line 166
> > <https://reviews.apache.org/r/45169/diff/2/?file=1382549#file1382549line166>
> >
> >     Changing this name may (or may not) impact UI.  The group name is not 
> > guaranteed unique, and some additional information may be displayed based 
> > on it's name (unconfirmed, but that's the intent).  Also, make consistent 
> > using an '_': SERVICE_CHECK_1

In order to be able to add a custom component into a specific service check 
group, something about them needs to be unique.  So either the name or the 
title.  I could change the title from "All Service Checks after core masters" 
or something like that.


- Tim


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


On May 16, 2016, 6:50 p.m., Tim Thorpe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45169/
> -----------------------------------------------------------
> 
> (Updated May 16, 2016, 6:50 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Jayush 
> Luniya, and Nate Cole.
> 
> 
> Bugs: AMBARI-15388
>     https://issues.apache.org/jira/browse/AMBARI-15388
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Currently the upgrade is defined as a series of xml files specific to the 
> current stack version and the target stack version. Each upgrade xml defines 
> the overall sequence of the upgrade and what needs to be done for each 
> service. It would both easier to maintain and easier to add new services, if 
> the services themselves could specify what should be done during their 
> upgrade.
> 
> There are two ways to make these changes, the alternate approach would be to 
> only make the java changes and not split the upgrade xml files.  This would 
> still allow new services to add themselves into the upgrade.  The benefit of 
> this is that for the stack services you only have one upgrade xml file.  The 
> problem with that is it is easier for a particular service to have 
> unintentional changes between upgrade xml files.
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/CommonServiceDirectory.java
>  7f7a49e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceDirectory.java
>  8a7b42b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/ServiceModule.java 
> f781574 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java
>  13d5047 
>   ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java 
> 6129ec0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/stack/StackServiceDirectory.java
>  88f6e19 
>   ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java 
> 43cefb9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/UpgradePack.java
>  b860731 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
>  67d7fdb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ServiceCheckGrouping.java
>  5cda422 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml 
> 6b74af0 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.4.xml 
> 9fb2bba 
>   ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.5.xml 
> 1cd2ffa 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.4.xml 
> e3bc7a3 
>   ambari-server/src/main/resources/stacks/HDP/2.4/upgrades/upgrade-2.5.xml 
> 9c6a02d 
>   ambari-server/src/main/resources/stacks/HDP/2.5/upgrades/upgrade-2.5.xml 
> 1745de8 
> 
> Diff: https://reviews.apache.org/r/45169/diff/
> 
> 
> Testing
> -------
> 
> Manual testing so far.  I have the code read the upgrade xml and all of its 
> service specific xml files, built the upgrade pack and then write the full 
> upgrade xml to disk and then compare the results to the original upgrade xml.
> 
> This review is mostly for the design doc which is attached to the JIRA.  Not 
> sure how to create a review board with a design doc instead of a patch file.
> 
> 
> Thanks,
> 
> Tim Thorpe
> 
>

Reply via email to