Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-03-22 Thread Tim Thorpe
> On March 22, 2016, 6:27 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/ModuleFileUnmarshaller.java, > > line 31 > > > > > > Please give me some time to go through th

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-03-22 Thread Tim Thorpe
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/ --- (Updated March 22, 2016, 6:40 p.m.) Review request for Ambari, Alejandro Fernan

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-03-29 Thread Nate Cole
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review125862 --- I think you need a more concrete way of ordering here. What if tw

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-03-29 Thread Tim Thorpe
> On March 29, 2016, 12:49 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java, > > lines 535-547 > > > > > > What exactly is the use case for marshalling back t

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-04-05 Thread Jayush Luniya
> On March 29, 2016, 12:49 p.m., Nate Cole wrote: > > I think you need a more concrete way of ordering here. What if two > > services are marked as YARN? Which one takes precedence? You may > > want to introduce an in order to > > specifically state how it happens. Order would be a float

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-04-06 Thread Tim Thorpe
> On March 29, 2016, 12:49 p.m., Nate Cole wrote: > > I think you need a more concrete way of ordering here. What if two > > services are marked as YARN? Which one takes precedence? You may > > want to introduce an in order to > > specifically state how it happens. Order would be a float

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-04-06 Thread Nate Cole
> On March 29, 2016, 8:49 a.m., Nate Cole wrote: > > I think you need a more concrete way of ordering here. What if two > > services are marked as YARN? Which one takes precedence? You may > > want to introduce an in order to > > specifically state how it happens. Order would be a float,

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-04-06 Thread Tim Thorpe
> On March 29, 2016, 12:49 p.m., Nate Cole wrote: > > I think you need a more concrete way of ordering here. What if two > > services are marked as YARN? Which one takes precedence? You may > > want to introduce an in order to > > specifically state how it happens. Order would be a float

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-13 Thread Tim Thorpe
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/ --- (Updated May 13, 2016, 1:46 p.m.) Review request for Ambari, Alejandro Fernande

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-16 Thread Jayush Luniya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review133499 --- Can you add unit test coverage? - Jayush Luniya On May 16, 2016

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-16 Thread Jayush Luniya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review133501 --- ambari-server/src/main/java/org/apache/ambari/server/stack/StackM

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Jayush Luniya
> On May 17, 2016, 6:33 a.m., Jayush Luniya wrote: > > Can you add unit test coverage? We should have unit tests in particular to validate incorrectly authored service upgrade packs. What happens if we add a circular dependency (example: KAFKA is marked with KNOX and KNOX is marked with KAFKA

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Jayush Luniya
> On May 17, 2016, 6:33 a.m., Jayush Luniya wrote: > > Can you add unit test coverage? > > Jayush Luniya wrote: > We should have unit tests in particular to validate incorrectly authored > service upgrade packs. What happens if we add a circular dependency (example: > KAFKA is marked with

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Tim Thorpe
> On May 17, 2016, 6:41 a.m., Jayush Luniya wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > line 685 > > > > > > UGM? This was just for testing purposes. I'll remove the

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Nate Cole
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review133533 --- ambari-server/src/main/java/org/apache/ambari/server/stack/StackM

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Jayush Luniya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review133556 --- ambari-server/src/main/java/org/apache/ambari/server/stack/StackM

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Jayush Luniya
> On May 17, 2016, 5:39 p.m., Jayush Luniya wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > line 844 > > > > > > The after tag is overloaded. Meaning it could mean insert

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Alejandro Fernandez
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review133587 --- ambari-server/src/main/java/org/apache/ambari/server/stack/StackD

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Tim Thorpe
> On May 17, 2016, 6:23 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml, > > line 166 > > > > > > Does the group name have to be unique now or is this on

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Tim Thorpe
> On May 17, 2016, 6:33 a.m., Jayush Luniya wrote: > > Can you add unit test coverage? > > Jayush Luniya wrote: > We should have unit tests in particular to validate incorrectly authored > service upgrade packs. What happens if we add a circular dependency (example: > KAFKA is marked with

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Tim Thorpe
> On May 17, 2016, 2:30 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > lines 890-892 > > > > > > Really need to throw here or just log it and continue; ?

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Tim Thorpe
> 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 > > > > > > With no other exceptions, can just use "regular

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Tim Thorpe
> On May 17, 2016, 6:23 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackDirectory.java, > > line 485 > > > > > > May want to check that upgradeFile is not null be

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Tim Thorpe
> On May 17, 2016, 6:23 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml, > > line 166 > > > > > > Does the group name have to be unique now or is this on

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Tim Thorpe
> On May 17, 2016, 5:39 p.m., Jayush Luniya wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > line 844 > > > > > > The after tag is overloaded. Meaning it could mean insert

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Tim Thorpe
> On May 17, 2016, 2:30 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > lines 822-831 > > > > > > Visitor or abstract method? Added the mergeRegularGroupi

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Tim Thorpe
> On May 17, 2016, 5:39 p.m., Jayush Luniya wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > line 829 > > > > > > What about RestartGrouping, ColocatedGrouping, StartGroupi

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-17 Thread Nate Cole
> On May 17, 2016, 2:23 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/resources/stacks/HDP/2.3/upgrades/upgrade-2.3.xml, > > line 166 > > > > > > Does the group name have to be unique now or is this on

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-18 Thread Tim Thorpe
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/ --- (Updated May 18, 2016, 1:19 p.m.) Review request for Ambari, Alejandro Fernande

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-18 Thread Nate Cole
> On May 17, 2016, 10:30 a.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > lines 890-892 > > > > > > Really need to throw here or just log it and continue; ?

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-18 Thread Tim Thorpe
> On May 17, 2016, 2:30 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > lines 890-892 > > > > > > Really need to throw here or just log it and continue; ?

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-18 Thread Nate Cole
> On May 17, 2016, 10:30 a.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > lines 890-892 > > > > > > Really need to throw here or just log it and continue; ?

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-18 Thread Tim Thorpe
> On May 17, 2016, 2:30 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > lines 890-892 > > > > > > Really need to throw here or just log it and continue; ?

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-18 Thread Tim Thorpe
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/ --- (Updated May 18, 2016, 6:26 p.m.) Review request for Ambari, Alejandro Fernande

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-18 Thread Jayush Luniya
> On May 17, 2016, 5:39 p.m., Jayush Luniya wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > line 844 > > > > > > The after tag is overloaded. Meaning it could mean insert

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-18 Thread Jayush Luniya
> On May 17, 2016, 6:33 a.m., Jayush Luniya wrote: > > Can you add unit test coverage? > > Jayush Luniya wrote: > We should have unit tests in particular to validate incorrectly authored > service upgrade packs. What happens if we add a circular dependency (example: > KAFKA is marked with

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-19 Thread Tim Thorpe
> On May 17, 2016, 5:39 p.m., Jayush Luniya wrote: > > ambari-server/src/main/java/org/apache/ambari/server/stack/StackModule.java, > > line 844 > > > > > > The after tag is overloaded. Meaning it could mean insert

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-19 Thread Tim Thorpe
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/ --- (Updated May 19, 2016, 3:23 p.m.) Review request for Ambari, Alejandro Fernande

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-19 Thread Alejandro Fernandez
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review134000 --- Ship it! Ship It! - Alejandro Fernandez On May 19, 2016, 3:2

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-19 Thread Jayush Luniya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review134014 --- Ship it! Ship It! - Jayush Luniya On May 19, 2016, 3:23 p.m.

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-19 Thread Jayush Luniya
> On May 19, 2016, 7:31 p.m., Jayush Luniya wrote: > > Ship It! Committed patch in trunk and branch-2.4 - Jayush --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review134014 --

Re: Review Request 45169: AMBARI-15388 - Upgrade XML should be pushed down as much as possible to the services

2016-05-23 Thread Nate Cole
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45169/#review134336 --- It looks like this was pushed, is there a need to keep the review