> On 2011-12-08 04:49:00, Mohamed Battisha wrote:
> > /trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java, 
> > line 200
> > <https://reviews.apache.org/r/3069/diff/1/?file=63126#file63126line200>
> >
> >     change the header to reflect the newly added authentication
> >     
> >     as well add the new exception you are currently throwing .. 
> > [IllegalArgumentException]

Header is changed.

The exception 'IllegalArgumentException' is convert to OozieClientException, so 
there is no need to write it explicitly.


> On 2011-12-08 04:49:00, Mohamed Battisha wrote:
> > /trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java, 
> > line 199
> > <https://reviews.apache.org/r/3069/diff/1/?file=63126#file63126line199>
> >
> >     Typo .. instantiated .

fixed.


> On 2011-12-08 04:49:00, Mohamed Battisha wrote:
> > /trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java, 
> > line 98
> > <https://reviews.apache.org/r/3069/diff/1/?file=63126#file63126line98>
> >
> >     why you want to print the tokens ?
> >

removed.


> On 2011-12-08 04:49:00, Mohamed Battisha wrote:
> > /trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java, 
> > line 220
> > <https://reviews.apache.org/r/3069/diff/1/?file=63126#file63126line220>
> >
> >     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 ..

fixed.


> On 2011-12-08 04:49:00, Mohamed Battisha wrote:
> > /trunk/client/src/main/java/org/apache/oozie/client/AuthOozieClient.java, 
> > line 231
> > <https://reviews.apache.org/r/3069/diff/1/?file=63126#file63126line231>
> >
> >     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

fixed.


- Angelo K.


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


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