> On Dec. 5, 2014, 3:22 p.m., Tom Beerbower wrote: > > Looks good. A handful of comments... mostly minor stuff or things that we > > can cleanup in a follow up Jira.
Thanks for reviewing! Will address... > On Dec. 5, 2014, 3:22 p.m., Tom Beerbower wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeGroupResourceProvider.java, > > line 182 > > <https://reviews.apache.org/r/28759/diff/1/?file=783784#file783784line182> > > > > Why don't you like autoboxing? It feels like cheating :) > On Dec. 5, 2014, 3:22 p.m., Tom Beerbower wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java, > > line 58 > > <https://reviews.apache.org/r/28759/diff/1/?file=783785#file783785line58> > > > > Would be good if we could extend StageResourceProvider but we can look > > at that later. Agreed. I was having difficulty with property definitions and AMC for these > On Dec. 5, 2014, 3:22 p.m., Tom Beerbower wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java, > > line 87 > > <https://reviews.apache.org/r/28759/diff/1/?file=783785#file783785line87> > > > > hmmm... just thinking out loud. Would it be so bad to leave them as > > stage properties or do we really want to hide the fact that an upgrade item > > is a stage? At the time, they were separate. We could make them look like Request->UpgradeGroup->Stage, but didn't look as "natural" as Upgrade->UpgradeGroup->UpgradeItem. This API will start as unpublished, so we can change semantics later. - Nate ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28759/#review64060 ----------------------------------------------------------- On Dec. 5, 2014, 11:56 a.m., Nate Cole wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28759/ > ----------------------------------------------------------- > > (Updated Dec. 5, 2014, 11:56 a.m.) > > > Review request for Ambari, Alejandro Fernandez and Tom Beerbower. > > > Bugs: AMBARI-8539 > https://issues.apache.org/jira/browse/AMBARI-8539 > > > Repository: ambari > > > Description > ------- > > Ties Upgrade->Request and UpgradeItem->Stage more closely. The properties of > the Request/Stage are pulled in, but assigned to the Upgrade/UpgradeItem > response objects. This was preferred over having 2 "top level" objects > (which didn't work with predicates anyway). > > The method of pulling in the other objects is a bit unorthodox, and should be > revisited when we have time. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java > b5fe94e > > ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeGroupService.java > da21658 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java > fb77853 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java > 8b66491 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java > 561f5d9 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java > 27ce015 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeGroupResourceProvider.java > b7eb240 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java > c481dae > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java > a2c30f7 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java > c08f809 > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java > 4320dbe > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java > fbb4f43 > > Diff: https://reviews.apache.org/r/28759/diff/ > > > Testing > ------- > > Tests run: 2323, Failures: 0, Errors: 0, Skipped: 18 > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 18:05.831s > [INFO] Finished at: Fri Dec 05 08:29:10 PST 2014 > [INFO] Final Memory: 30M/321M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Nate Cole > >
