hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thejas Nair" <the...@hortonworks.com>
Subject Re: Review Request 17437: doAs with plain sasl auth should be session aware
Date Thu, 13 Mar 2014 12:38:35 GMT

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



service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java
<https://reviews.apache.org/r/17437/#comment68299>

    I figure you have moved the functions that don't need to be executed under doAs() to this
base interface. Can you also add a class comment about that (ie only functions that need not
be executed under doAs action should be put here)?
    



service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
<https://reviews.apache.org/r/17437/#comment68300>

    can you add @Override annotation ?
    



service/src/java/org/apache/hive/service/cli/session/SessionManager.java
<https://reviews.apache.org/r/17437/#comment68301>

    Having the threadlocal username and ipaddress in TSetIpAddressProcessor looks fine to
me. We don't seem to gain much by having the extra hop in SessionManager. TSetIpAddressProcessor
is the only class that sets/resets it.
    I agree that this can break other pending patches, but we can hopefully get this in quickly.
    


- Thejas Nair


On Jan. 28, 2014, 1:21 a.m., Navis Ryu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17437/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2014, 1:21 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6312
>     https://issues.apache.org/jira/browse/HIVE-6312
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> TUGIContainingProcessor creates new Subject for each invocation which induces FileSystem
leakage when cache is enable(true by default).
> 
> 
> Diffs
> -----
> 
>   service/src/java/org/apache/hive/service/auth/PlainSaslHelper.java 15b1675 
>   service/src/java/org/apache/hive/service/auth/TSetIpAddressProcessor.java 0bf34ce 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java c8fb8ec 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java PRE-CREATION

>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 445c858 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionProxy.java 76f18a9

>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java bfe0e7b 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java b5a6138 
>   service/src/test/org/apache/hive/service/auth/TestPlainSaslHelper.java 8fa4afd 
> 
> Diff: https://reviews.apache.org/r/17437/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Navis Ryu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message