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


I guess that I don't really understand the issue.  If I'm correct desired_state 
is what we are trying to set the state of the host component to.  In the 
original query from the Jira ...

api/v1/clusters/<clustername>/host_components?HostRoles/state=INSTALL_FAILED

The property in the predicate is HostRoles/state=INSTALL_FAILED.  Since we are 
not setting state, we shouldn't have to map anything here.  All we care about 
is that only host components where HostRoles/state=INSTALL_FAILED are returned.

I think that its a mistake that we make the conversion when we build the 
request ...

    ServiceComponentHostRequest serviceComponentHostRequest = new 
ServiceComponentHostRequest(
        (String) properties.get(HOST_COMPONENT_CLUSTER_NAME_PROPERTY_ID),
        (String) properties.get(HOST_COMPONENT_SERVICE_NAME_PROPERTY_ID),
        (String) properties.get(HOST_COMPONENT_COMPONENT_NAME_PROPERTY_ID),
        (String) properties.get(HOST_COMPONENT_HOST_NAME_PROPERTY_ID),
        (String) properties.get(HOST_COMPONENT_STATE_PROPERTY_ID));

The last parameter is desired state but we pass state.  I think that we should 
pass properties.get(HOST_COMPONENT_DESIRED_STATE_PROPERTY_ID) there, which I 
think would make your query pass.

I think that the only place where we may want to convert from state to desired 
state is in updateResources and createResources in 
HostComponentResourceProvider.  In these cases the request.getProperties() is 
used to build the serviceComponentHostRequest, not the predicate. Can we just 
check the map of properties used to create the request there and do the 
conversion if needed in that map?
 

- Tom Beerbower


On March 4, 2014, 4:57 p.m., Dmitro Lisnichenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18708/
> -----------------------------------------------------------
> 
> (Updated March 4, 2014, 4:57 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-4782
>     https://issues.apache.org/jira/browse/AMBARI-4782
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> The general idea is to replace "state" property at user predicate (for 
> HostComponent update requests) with desired_state to comply with current 
> usage and keep all hack in one place. This is done at UpdateHandler. 
> We can not do that later, because request type information (GET or PUT) is 
> not available at this time. Changing url parameters before compiling 
> predicate seems more hacky for me. That's why I implemented a visitor that 
> iterates over predicate and replaces properties. The code that is executed 
> afterwards transparently works with "desired_state" property instead of 
> "state" property. Get requests are processed at natural way, "state" is 
> mapped to live state for all requests except update requests.
> 
> Need more work on replacing "state" property in request body.
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/handlers/BaseManagementHandler.java
>  c34f0d7 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/handlers/UpdateHandler.java
>  338d411 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  10d07b6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceComponentHostRequest.java
>  d9c7928 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java
>  23eafcb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java
>  89d53ae 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ReplacePropertyPredicateVisitor.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
>  1e402eb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/predicate/PropertyPredicate.java
>  5715d2a 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PredicateHelper.java
>  381fcac 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
>  c99bfa1 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
>  dcee4bf 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractResourceProviderTest.java
>  11adbee 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java
>  ec82e55 
> 
> Diff: https://reviews.apache.org/r/18708/diff/
> 
> 
> Testing
> -------
> 
> Here is a preview version of patch (without unit tests).  Not wll-tested 
> end2end yet.
> 
> 
> Thanks,
> 
> Dmitro Lisnichenko
> 
>

Reply via email to