ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tom Beerbower" <tbeerbo...@hortonworks.com>
Subject Re: Review Request 30077: Upgrades: Make a ResourceDefinition for UpgradeResourceProvider for downgrade directive
Date Tue, 20 Jan 2015 18:41:44 GMT


> 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
> 
>


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