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