Mailing-List: contact dev-help@ambari.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@ambari.apache.org
Content-Type: multipart/alternative;
 boundary="===============0029079014001572954=="
MIME-Version: 1.0
Subject: Re: Review Request 31945: Identity references fail to deference for
 service-level references in Kerberos descriptor parser
From: "John Speidel" <jspeidel@hortonworks.com>
To: "Emil Anca" <eanca@hortonworks.com>,
 "John Speidel" <jspeidel@hortonworks.com>,
 "Sumit Mohanty" <smohanty@hortonworks.com>
Cc: "Robert Levas" <rlevas@hortonworks.com>, "Ambari" <dev@ambari.apache.org>
Date: Wed, 11 Mar 2015 20:03:42 -0000
Message-ID: <20150311200342.14616.82913@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "John Speidel" <noreply@reviews.apache.org>
References: <20150311192331.14616.35387@reviews.apache.org>
In-Reply-To: <20150311192331.14616.35387@reviews.apache.org>
Reply-To: "John Speidel" <jspeidel@hortonworks.com>

--===============0029079014001572954==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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

Ship it!


ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/AbstractKerberosDescriptorContainer.java
<https://reviews.apache.org/r/31945/#comment123539>

    confusing?


ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/AbstractKerberosDescriptorContainer.java
<https://reviews.apache.org/r/31945/#comment123540>

    Using instanceof here to check for concrete type is ugly and against OOP design principals.  Consider something like the template method pattern. 
    
    Since this code isn't functionally incorrect, I have made this a comment only and will leave it up to you whether you want to change it.


- John Speidel


On March 11, 2015, 7:23 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31945/
> -----------------------------------------------------------
> 
> (Updated March 11, 2015, 7:23 p.m.)
> 
> 
> Review request for Ambari, Emil Anca, John Speidel, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-9007
>     https://issues.apache.org/jira/browse/AMBARI-9007
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Identity references fail to deference for service-level references in Kerberos descriptor parser.  For example:
> 
> ````
> /SERVICE/identity
> ````
> 
> Global references appear to work. Example:
> 
> ```
> /identity
> ```
> 
> The solution is to properly parse the path to the identity and handle the different paths types:
> 
> - /identity - stack-level identity
> - /SERVICE/identity - service-level identity
> - /SERVICE/COMPONENT/identity - component-level identity
> - ./identity - service-relative-level identity
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/AbstractKerberosDescriptorContainer.java 2ec2cb5 
>   ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosDescriptor.java 791b0e6 
>   ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/kerberos.json ad5ee3e 
>   ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorTest.java 97f8008 
>   ambari-server/src/test/resources/kerberos/test_get_referenced_identity_descriptor.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31945/diff/
> 
> 
> Testing
> -------
> 
> Manual testing
> 
> #Local test results
> Running org.apache.ambari.server.state.kerberos.KerberosDescriptorTest
> Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.217 sec
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 39:59.216s
> [INFO] Finished at: Wed Mar 11 13:53:09 EDT 2015
> [INFO] Final Memory: 58M/823M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


--===============0029079014001572954==--