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


Looks good

Few questions/comments/recommendations


/trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java
<https://reviews.apache.org/r/3069/#comment8389>

    why you want to print the tokens ?
    



/trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java
<https://reviews.apache.org/r/3069/#comment8393>

    Typo .. instantiated .



/trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java
<https://reviews.apache.org/r/3069/#comment8396>

    change the header to reflect the newly added authentication
    
    as well add the new exception you are currently throwing .. 
[IllegalArgumentException]



/trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java
<https://reviews.apache.org/r/3069/#comment8394>

    this piece of code needs to be simplified ...
    a switch statement may be more suitable here
    
    return (authType ==AuthType.KERBEROS) ?  new KerberosAuthenticator() : new 
PseudoAuthenticator();
    
    
    you already checked for the type while getting the type of authType ..



/trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java
<https://reviews.apache.org/r/3069/#comment8395>

    catching a generic exception is not always good programming practice..
    it is better to spill out the type of exceptions you may need to catch



/trunk/core/src/main/conf/oozie-site.xml
<https://reviews.apache.org/r/3069/#comment8388>

    extra tab


- Mohamed


On 2011-12-08 02:52:04, Angelo K. Huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3069/
> -----------------------------------------------------------
> 
> (Updated 2011-12-08 02:52:04)
> 
> 
> Review request for oozie.
> 
> 
> Summary
> -------
> 
> This improvement is mainly to add client parameter options to handle user 
> specified authentication option. In Oozie-77, client authentication uses 
> fall-back strategy to handle authentication, such as kerberos -> simple. User 
> should allow to give parameter or property to specify which authentication to 
> use.
> 
> A proposal is :
> 
> -auth simple
> -auth kerberos
> -auth <auth_name> 
> 
> 
> This addresses bug OOZIE-624.
>     https://issues.apache.org/jira/browse/OOZIE-624
> 
> 
> Diffs
> -----
> 
>   /trunk/client/src/main/java/org/apache/oozie/cli/OozieCLI.java 1209346 
>   /trunk/docs/src/site/twiki/DG_CommandLineTool.twiki 1209346 
>   /trunk/core/src/main/conf/oozie-site.xml 1209346 
>   /trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java 
> 1209346 
> 
> Diff: https://reviews.apache.org/r/3069/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Angelo K.
> 
>

Reply via email to