> On Jan. 20, 2015, 6:26 p.m., Alejandro Fernandez wrote: > >
Thanks for the review! > On Jan. 20, 2015, 6:26 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java, > > line 39 > > <https://reviews.apache.org/r/30077/diff/2/?file=826962#file826962line39> > > > > Shouldn't this still pass the "validate_topology" directive? I doesn't look like the validate_topology directive is actually used for client configs. I think that it was included in the resource definition by accident when it was cut and pasted from the blueprint resource definition. Do you think or know for sure that the client configs uses this directive? I only see it used for blueprints. > On Jan. 20, 2015, 6:26 p.m., Alejandro Fernandez wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java, > > line 38 > > <https://reviews.apache.org/r/30077/diff/2/?file=826965#file826965line38> > > > > I assume you bit the bullet and used this approach so as to not assume > > that an upgrade/downgrade will always require service checks. I think this > > is more flexible :-) I think it makes it a little easier to constrct a resource definition. In fact, the only reason that I even added this class was because I thought it was the logical place to define the directive constants. Otherwise, I would have just constructed a SimpleResourceDefinition inline in the factory. - Tom ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30077/#review68747 ----------------------------------------------------------- On Jan. 20, 2015, 6:18 p.m., Tom Beerbower wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30077/ > ----------------------------------------------------------- > > (Updated Jan. 20, 2015, 6:18 p.m.) > > > Review request for Ambari, Jonathan Hurley and Nate Cole. > > > Bugs: AMBARI-9214 > https://issues.apache.org/jira/browse/AMBARI-9214 > > > Repository: ambari > > > Description > ------- > > Add an Upgrade ResourceDefinition so that we can use these directives: > > downgrade: boolean that makes the upgrade a downgrade > skip_service_checks: boolean that disables the upcoming service-check-all > after core components > > > Also cleaned up a sloppy cut and paste job in ClientConfigResourceDefinition. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java > 1cd7e17 > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ClientConfigResourceDefinition.java > 4ff37ac > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java > fc28c13 > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/SimpleResourceDefinition.java > 92ecd1e > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java > d9c9aec > > ambari-server/src/test/java/org/apache/ambari/server/api/resources/BaseResourceDefinitionTest.java > 73aa828 > > ambari-server/src/test/java/org/apache/ambari/server/api/resources/SimpleResourceDefinitionTest.java > d264511 > > ambari-server/src/test/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinitionTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java > cd5f23e > > Diff: https://reviews.apache.org/r/30077/diff/ > > > Testing > ------- > > Unit tests added. All existing tests pass. > > Manual tests in progress... > > > Thanks, > > Tom Beerbower > >
