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

Ship it!


Looks good.  Some minor stuff...


-  ClusterControllerImpl :

Very nit-picky ... If you are making this public, could you move it up with the 
other public methods?  Also, if it needs to be public should it be part of the 
ClusterController interface?

 public ResourceProvider ensureResourceProvider(Resource.Type type) {


-  RequestStages :

This is mostly a personal preference but I never liked naming a class as a 
plural.  If an instance of this object contains stages would it be clearer to 
call it something like RequestStageContainer or RequestStageList?


-  ServiceResourceProvider :

You added this new method.  Does it need to be package-private scope or could 
it be made private?  Could you move it down with the utility methods?

  RequestStages doUpdateResources(final RequestStages stages, final Request 
request, Predicate predicate)
      throws UnsupportedPropertyException, SystemException, 
NoSuchResourceException, NoSuchParentResourceException {


-  AmbariManagementControllerTest : 

+1 for cleaning this kind of thing up! ...
    catch (Exception e) {
      // Expected
      e.printStackTrace();
    }



- Tom Beerbower


On March 14, 2014, 12:01 a.m., John Speidel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19209/
> -----------------------------------------------------------
> 
> (Updated March 14, 2014, 12:01 a.m.)
> 
> 
> Review request for Ambari, Nate Cole, Sid Wagle, and Tom Beerbower.
> 
> 
> Bugs: AMBARI-5077
>     https://issues.apache.org/jira/browse/AMBARI-5077
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Provision a cluster from an Ambari Blueprint.
> Allow a cluster to be fully provisioned via the Ambari REST API with a single 
> REST API call by specifying a blueprint name. This means that from a single, 
> simple asynchronous REST API call, a cluster can now be provisioned from INIT 
> to STARTED.
> 
> For more see the corresponding Jira:
> https://issues.apache.org/jira/browse/AMBARI-5077
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java
>  03ddaa6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  5f85d83 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
>  5e2e47c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterControllerImpl.java
>  707c66a 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java
>  3d55871 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ComponentResourceProvider.java
>  59a52e5 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestStages.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
>  76a17d2 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/SystemException.java
>  0426657 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
>  c877ac0 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java
>  96a9814 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ComponentResourceProviderTest.java
>  76c6ae3 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestStagesTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
>  8bc97cd 
> 
> Diff: https://reviews.apache.org/r/19209/diff/
> 
> 
> Testing
> -------
> 
> Functional tests with a single host group and HDFS, OOZIE, NAGIOS and GANGLIA 
> services.
> 
> Unit tests added.
> Ran all existing unit tests:
> OK
> ----------------------------------------------------------------------
> Total run:507
> Total errors:0
> Total failures:0
> ...
> [INFO] Executed tasks
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 18:37.734s
> [INFO] Finished at: Thu Mar 13 19:13:12 EDT 2014
> [INFO] Final Memory: 28M/123M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> John Speidel
> 
>

Reply via email to