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 33079: Views : Auto create
Date Fri, 10 Apr 2015 14:56:59 GMT

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

Ship it!


No responses are necessary, but I added some notes and some nit-picky comments.


ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
<https://reviews.apache.org/r/33079/#comment129200>

    The ViewRegistry appears to be injected.  Do we also want to pass in the a ViewRegistry
via a constructor?  I suppose nothing is really wrong here.



ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java
<https://reviews.apache.org/r/33079/#comment129201>

    Couldn't this be another AND clause to the above if statement?  
    
    Also, if serviceName exists in serviceNames, wouldn't this check also include the check
for `autoCreateServices.contains(serviceName)`?



ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
<https://reviews.apache.org/r/33079/#comment129202>

    If this is the only usage of setting the ViewRegistry using the constructor, maybe use
`getDeclaredField` instead - like it is done for the injected KerberosHelper.



ambari-views/src/main/resources/view.xsd
<https://reviews.apache.org/r/33079/#comment129203>

    There is a `list` type in the XSD namespace that declares a _space_ delimited list of
values.  I don't think it really makes a difference here, but just a FYI.


- Robert Levas


On April 10, 2015, 10:08 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33079/
> -----------------------------------------------------------
> 
> (Updated April 10, 2015, 10:08 a.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley, John Speidel, and Robert Levas.
> 
> 
> Bugs: AMBARI-10424
>     https://issues.apache.org/jira/browse/AMBARI-10424
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Ability to automatically instantiate a view instance for a cluster during create based
on the stack and services included in the cluster – whether using blueprints or install
wizard (so views are created automatically with a cluster create). The view itself should
describe what stack and services it is associated with, and when a cluster is created in the
ambari the view is deploy in, a view instance is automatically created. Ambari Admins should
have an option to override this setting in case they do not want the view instances automatically
created. This includes support for designating in the view.xml stack and services support
for a view. Similar to the way min-ambari-version is specified, the view developer needs a
way to identify that the view works with stack(s) (such as "HDP-2.2" or "HDP-2.2, HDP 2.3"
or "HDP-2*") and what services need to be in the cluster ("YARN" or" YARN, HIVE"). Once a
cluster is created matching those stack + service requirements, the view is automat
 ically instantiated.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ServiceResourceProvider.java
2724a97 
>   ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java 1ae1dfd

>   ambari-server/src/main/java/org/apache/ambari/server/view/configuration/AutoInstanceConfig.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/view/configuration/ViewConfig.java
c617b7f 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ServiceResourceProviderTest.java
9ec1610 
>   ambari-server/src/test/java/org/apache/ambari/server/view/ViewRegistryTest.java 3a57b1b

>   ambari-server/src/test/java/org/apache/ambari/server/view/configuration/AutoInstanceConfigTest.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/view/configuration/ViewConfigTest.java
1875238 
>   ambari-views/src/main/resources/view.xsd b5ed669 
> 
> Diff: https://reviews.apache.org/r/33079/diff/
> 
> 
> Testing
> -------
> 
> Manual tested that adding service will trigger auto creation and that the new view instance
is associated with that cluster.
> New unit tests added.
> All tests pass.
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 34:51 min
> [INFO] Finished at: 2015-04-09T17:28:29-04:00
> [INFO] Final Memory: 52M/603M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


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