> On Dec. 8, 2014, 9:10 p.m., John Speidel wrote: > >
Thanks for the review. > On Dec. 8, 2014, 9:10 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariSessionManager.java, > > line 82 > > <https://reviews.apache.org/r/28785/diff/1/?file=784442#file784442line82> > > > > What does it mean to have a null session? Could the session be null > > when a user is trying to set a session attribute? If not, would this be an > > error case? If so, we just don't set the attribute and return back ok? I think that it's something that should never happen if this code is invoked through a servlet request. I guess that you could argue whether or not we should throw an exception if the session is null. I decided not to because the intent of the method is to set an attribute on the current session so that you could retrieve that attribute from the session as long as the session exists. If no session exists when you go to set the attribute then there is nothing to do. Again, I don't think it's something that should ever happen for our use cases. > On Dec. 8, 2014, 9:10 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java, > > line 400 > > <https://reviews.apache.org/r/28785/diff/1/?file=784445#file784445line400> > > > > property.substring(SESSION_ATTRIBUTES_PROPERTY_PREFIX.length()) would > > likely be faster as the index is already known Yep, I'll change it. Thanks. - Tom ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28785/#review64275 ----------------------------------------------------------- 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 > >
