ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robert Levas" <rle...@hortonworks.com>
Subject Re: Review Request 38182: Blueprint: API call to create cluster instance with blueprint fails when blueprint_name is not correct
Date Wed, 09 Sep 2015 16:36:42 GMT


> On Sept. 8, 2015, 5:14 p.m., Robert Nettleton wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/persistence/PersistenceManagerImpl.java,
line 73
> > <https://reviews.apache.org/r/38182/diff/1/?file=1065274#file1065274line73>
> >
> >     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.

I agree with @rnettleton, however I do like the way this solution attempts to disambiguate
a resource's name when this scenario occurs.  Wouldn't a more _local_ solution be to simply
assume that the name in the URL overrides the name in the JSON document?  Or is it too late
by the time Ambari knows that the resource being created it a Blueprint?


- Robert


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


On Sept. 9, 2015, 11:01 a.m., Sandor Magyari wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38182/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 11:01 a.m.)
> 
> 
> Review request for Ambari, Robert Levas and Robert Nettleton.
> 
> 
> 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 unit tests passed, it seems it doesn't affect normal functionality.
> 
> 
> Thanks,
> 
> Sandor Magyari
> 
>


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