> On Dec. 8, 2014, 12:03 p.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java,
> >  line 93
> > <https://reviews.apache.org/r/28785/diff/1/?file=784444#file784444line93>
> >
> >     Why is this one not like the others, where the property IDs are read 
> > from properties.json?  
> >     
> >     Personally I prefer it the way you have implemented it and I am not 
> > particularly keen on reading the properties from the `properties.json` file 
> > - though it may not be clear to me if there are other usages of the the 
> > `properties.json` file.

The properties files were part of the initial implementation.  The idea was 
that defining the properties in configuration would make the providers more 
flexible so that we could, for example, have a provider that exposes different 
properties in different stack definitions.  It never really worked out that way 
since many of the resource providers need knowledge of the specific properties 
that they expose ant can't treat them generically.  Since we have never 
actually made use of that flexibility I have been writing new and converting 
some of the existing resource providers to define their own properties.

The use of the properties files was never a requirement.  We have several 
patterns in place for implementing a resource provider but the only real 
requirements should be specified in the ResourceProvider interface.  The 
properties files do create some confusion so it may make sense to drop them 
altogether.  I would probably deprecate them if I could add comments to the 
json files.


- Tom


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


On Dec. 6, 2014, 5:54 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28785/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2014, 5:54 p.m.)
> 
> 
> Review request for Ambari, John Speidel and Robert Levas.
> 
> 
> Bugs: Ambari-8447
>     https://issues.apache.org/jira/browse/Ambari-8447
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Add API support to allow for session attributes to be set for a cluster.
> Expose the session_attributes property on a cluster resource. To add session 
> attributes for a cluster the session_attributes property can be set on a 
> cluster resource. The value of the session_attributes should be a collection 
> of name value pairs, as follows ...
> PUT api/v1/clusters/c1
> 
> {
>   "session_attributes" : {
>     "attr1" : "v1",
>     "attr2" : "v2",
>     "attr3" : {"sub1" : "v31", "sub2" : "v32"}
>   }
> }
> 
> 
> The above example would add the following attributes to the session for 
> cluster c1:
> attribute     value
> -----------------
> attr1     v1
> attr2     v2
> attr3/sub1    v31
> attr3/sub2    v32
> 
> 
> The cluster session attribute property is not available to read through the 
> REST API. The map of cluster session attribute properties is available to 
> resource providers on the Ambari server through the following method on the 
> org.apache.ambari.server.state.Clusters singleton...
> 
>     /**
>      * Get the map of session attributes for the cluster identified by the 
> given name.
>      * 
>      * @param name  the cluster name
>      *              
>      * @return the map of session attributes for the cluster
>      */
>     public Map<String, Object> getSessionAttributes(String name);
> 
> 
> All session attributes set through the above API are scoped by cluster name 
> and session. When the session dies, so do the attribute values for that 
> session.
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
>  abd83fc 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariSessionManager.java
>  e6dd07f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/ClusterRequest.java
>  8bbbd68 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java
>  6cb8fa4 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java
>  7423c25 
>   ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java 
> 125b71b 
>   ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java 
> 18f3a94 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
>  5a9731a 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
>  d2c7428 
>   
> ambari-server/src/test/java/org/apache/ambari/server/agent/AgentResourceTest.java
>  bce66ee 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
>  4b19443 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariSessionManagerTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java
>  f382588 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java
>  a11dc43 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterImplTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClustersImplTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/28785/diff/
> 
> 
> Testing
> -------
> 
> Added new unit tests.  Manual testing for session attributes.
> 
> All tests pass ...
> 
> Results :
> 
> Tests run: 2335, Failures: 0, Errors: 0, Skipped: 22
> 
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 25:09 min
> [INFO] Finished at: 2014-12-06T11:54:15-05:00
> [INFO] Final Memory: 30M/265M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>

Reply via email to