ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Nettleton" <rnettle...@hortonworks.com>
Subject Re: Review Request 38182: Blueprint: API call to create cluster instance with blueprint fails when blueprint_name is not correct
Date Tue, 08 Sep 2015 21:14:32 GMT

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


I think we should consider a different approach to fix this issue.  My concerns are listed
in the issue below.  

Thanks.


ambari-server/src/main/java/org/apache/ambari/server/api/services/persistence/PersistenceManagerImpl.java
(line 73)
<https://reviews.apache.org/r/38182/#comment154275>

    I'm a little concerned with this modification, as it will affect basically every resource
type managed by Ambari.  
    
    We probably should not make a change that could potentially affect API compatibility in
the "v1" version of the API.  
    
    While it would be a less-generic solution, I think I would recommend building a solution
that is specific to the Blueprints issue, to reduce the risk of creating an API incompatibility
for all resource types managed by ambari-server. 
    
    John Speidel should probably comment on this further, since he was the lead on the API
services in Ambari.


- Robert Nettleton


On Sept. 8, 2015, 3:57 p.m., Sandor Magyari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38182/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2015, 3:57 p.m.)
> 
> 
> Review request for Ambari, John Speidel, Robert Nettleton, and Jeff Sposetti.
> 
> 
> Bugs: AMBARI-12989
>     https://issues.apache.org/jira/browse/AMBARI-12989
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> PROBLEM
> 
> When creating any resource like blueprint, cluster etc, you can specify the resource
name both in the Url and Json payload as well. For example in case of a blueprint creation:
api/v1/blueprints/sandboxURL Json: {"Blueprints":{"blueprint_name":"sandboxJson"}}. In this
case the blueprint will be created as sandboxJson so the name specified in Json will override
the name specifie din the Url. The behavoir is true for cluster creation and any other resource
where you can specify name in Json as well. This could be a bit misleading so the API may
return an error in case of resource names don't match.
> 
> SOLUTION
> 
> The solution would be to throw an error message (Error code 400) in case resource names
are different. To achive this only for cluster creation would be quite complicated since resource
handling is quite generic and in ClusterResourceProvider - which contains resource specific
handling - only the request body is available, the url is not. The actual behaviour is that
in PersistenceManagerImpl.create method the resource name (eg. blueprint_name) is inserted
from the url in Json in case it's missing. So if resource name is specified in url than that
will be used, if is specified both url and Json then latter will be used. Because of this
behaviour I think we should hadnle this problem in PersistenceManagerImpl.create as you can
see in attached patch. This will affect of course the creation of all resources. 
> All unittests were
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/persistence/PersistenceManagerImpl.java
4db5611 
>   ambari-server/src/test/java/org/apache/ambari/server/api/services/PersistenceManagerImplTest.java
9ff1506 
> 
> Diff: https://reviews.apache.org/r/38182/diff/
> 
> 
> Testing
> -------
> 
> Manually tested blueprint and cluster creation, then cluster creation with blueprint.
All unitests passed, it seems it doesn't affect normal functionality.
> 
> 
> Thanks,
> 
> Sandor Magyari
> 
>


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