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

Ship it!



ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java
<https://reviews.apache.org/r/32075/#comment124029>

    Could you have done this instead and not had to change the Cluster 
interface? ...
    
          private void setKerberosServiceCheckIdentifier(Cluster cluster, 
String value) {
            Map<String, Object> sessionAttributes = new 
HashMap(cluster.getSessionAttributes());
        
            if (value == null) {
              sessionAttributes.remove(SERVICE_CHECK_IDENTIFIER);
            } else {
              sessionAttributes.put(SERVICE_CHECK_IDENTIFIER, value);
            }
        
            cluster.addSessionAttributes(sessionAttributes);
          }
          
    Since this is for 2.0.0, it might be better to make minimal changes.
    
    Otherwise, I don't have an issue with the additions.  You can decide which 
path is less risky.


- Tom Beerbower


On March 14, 2015, 11:13 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32075/
> -----------------------------------------------------------
> 
> (Updated March 14, 2015, 11:13 p.m.)
> 
> 
> Review request for Ambari, Emil Anca, John Speidel, Mahadev Konar, Sid Wagle, 
> and Tom Beerbower.
> 
> 
> Bugs: AMBARI-10071
>     https://issues.apache.org/jira/browse/AMBARI-10071
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> 1. Create cluster
> 2. Enable security
> 3. Logout or restart Ambari server
> 4. Go to Kerberos service page
> 5. Click service actions
> 6. Click Run Service Check
> 7. Service check does not run
> 
> #Solution
> The problem occurred because the map of attrbutes returned from the 
> `org.apache.ambari.server.state.Cluster#getSessionAttributes` was an 
> unmutable empty map if the relevant cluster's session attributes were never 
> set - if prevously set, a mutable map of attrobutes was returned.  This 
> causes an exception to be thrown when attempting to update the session 
> attributes if the KDC administrative credentials were not set in session.  
> 
> To fix this, methods were added to the Cluster interface to allow for 
> attributes to be explictly set or removed.
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java
>  cf73236 
>   ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java 
> 9b424b8 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
>  638e4da 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
>  c2ded8c 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterImplTest.java
>  8d07b9c 
> 
> Diff: https://reviews.apache.org/r/32075/diff/
> 
> 
> Testing
> -------
> 
> Manutally tested to see the session attributes updated as expected without 
> exceptions being thrown.
> 
> #Local test Results:
> Running org.apache.ambari.server.controller.KerberosHelperTest
> Tests run: 25, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.452 sec
> 
> Running org.apache.ambari.server.state.cluster.ClusterImplTest
> Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.389 sec
> 
> #Jenkins test Results:
> Failed due to `Could not find artifact 
> org.apache.ambari:ambari-metrics:pom:2.0.0-SNAPSHOT in oss.sonatype.org`
> 
> 
> Thanks,
> 
> Robert Levas
> 
>

Reply via email to