-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17523/#review33217
-----------------------------------------------------------
(I wish this tool allowed you to review by making comments on the diff.)
Some comments ...
1) In BlueprintResourceProvider, is this method needed?
Request toRequest(Predicate predicate)
The convention is that an empty request means get all fields (like select *).
Usually the provider calls a BaseProvider method to get the requested ids...
Set<String> requestedIds = getRequestPropertyIds(request, predicate);
... this should extract the property ids from the predicate and account for an
empty request.
2) I think that the BlueprintResourceProvider should extend
AbstractResourceProvider and not AbstractControllerResourceProvider.
AbstractControllerResourceProvider was intended for resource providers that use
an AmbariManagementController. You can move the BlueprintResourceProvider case
in the switch statement to AbstractResourceProvider.
3) Not sure your logic in BlueprintResourceProvider.getResources() is correct.
If you have a predicate like 'blueprint_name=FOO | blueprint_name=BAR',
getPropertyMaps(predicate) will generate a set of 2 maps ...
{blueprint_name:FOO} and {blueprint_name:BAR}. It looks like you would break
out after hitting the first name and only return a single blueprint. You can
probably remove the break and build a result set (or we can talk about other
ways to handle this).
4) Not sure if it was intentional but some methods in BlueprintDAO have Java
docs and others do not.
- Tom Beerbower
On Jan. 30, 2014, 2:08 a.m., John Speidel wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17523/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 2:08 a.m.)
>
>
> Review request for Ambari, Mahadev Konar, Sumit Mohanty, Tom Beerbower, and
> Yusaku Sako.
>
>
> Bugs: AMBARI-4467
> https://issues.apache.org/jira/browse/AMBARI-4467
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Create a new /blueprints REST endpoint. This endpoint represents an 'abstract
> blueprint' or 'template' and doesn't contain cluster specific details such
> specific host information.
>
> This initial jira will be limited to basic blueprint information and will not
> contain configuration elements. These additional elements will be added in
> subsequent patches.
>
> Available operations are get, create and delete. Update is not supported
> because blueprints are immutable.
>
> See the associated Apache Jira for an example of a blueprint resource.
>
>
> Diffs
> -----
>
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/BlueprintResourceDefinition.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java
> 7dcaccb
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/BlueprintService.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
> 75b6cb1
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java
> c66ae65
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java
> 52f0cdf
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/BlueprintDAO.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/BlueprintEntity.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntity.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntityPK.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupEntity.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupEntityPK.java
> PRE-CREATION
> ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 8b8e285
> ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 3f1a080
> ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 78dce48
> ambari-server/src/main/resources/META-INF/persistence.xml 3b07bd7
> ambari-server/src/main/resources/key_properties.json dc7e23a
> ambari-server/src/main/resources/properties.json 4a824d3
> ambari-server/src/main/resources/upgrade/ddl/Ambari-DDL-MySQL-UPGRADE.sql
> 509d1cf
> ambari-server/src/main/resources/upgrade/ddl/Ambari-DDL-Oracle-UPGRADE.sql
> a74f2a2
>
> ambari-server/src/main/resources/upgrade/ddl/Ambari-DDL-Postgres-UPGRADE-1.3.0.sql
> 674d08c
>
> ambari-server/src/test/java/org/apache/ambari/server/api/resources/BlueprintResourceDefinitionTest.java
> PRE-CREATION
>
> ambari-server/src/test/java/org/apache/ambari/server/api/services/BlueprintServiceTest.java
> PRE-CREATION
>
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintResourceProviderTest.java
> PRE-CREATION
>
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/BlueprintDAOTest.java
> PRE-CREATION
>
> ambari-server/src/test/java/org/apache/ambari/server/orm/entities/BlueprintEntityTest.java
> PRE-CREATION
>
> ambari-server/src/test/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntityTest.java
> PRE-CREATION
>
> ambari-server/src/test/java/org/apache/ambari/server/orm/entities/HostGroupEntityTest.java
> PRE-CREATION
>
> Diff: https://reviews.apache.org/r/17523/diff/
>
>
> Testing
> -------
>
> Added unit tests for new code.
> All unit tests pass.
> Functional tests included creating, getting and deleting blueprint resources.
>
>
> Thanks,
>
> John Speidel
>
>
