hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Carl Steinbach" <c...@cloudera.com>
Subject Re: Review Request: Pass user identity in metastore connection in unsecure mode
Date Mon, 19 Dec 2011 22:09:29 GMT

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


Is it possible to add some testcases? Specifically I'd like to see a test that has a client
with setugi enabled connect to a server with setugi disabled and vice-versa.



trunk/conf/hive-default.xml.template
<https://reviews.apache.org/r/2975/#comment9004>

    This describes the action instead of the effect. Please change to something like "In unsecure
mode, setting this property to true will cause the metastore to execute DFS operations using
the client's reported user and group permissions. Note that this property must be set on both
the client and server sides."
    
    Also, it may be easier to understand if you separate this out into to separate properties:
hive.metastore.client.setugi and hive.metastore.server.setugi



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/2975/#comment9005>

    Spacing.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/2975/#comment9000>

    The formatting/indentation in this method is still not correct. Please use 2 character
indents, nested 'else' operators, etc.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/2975/#comment9009>

    Please add some logging statements here, e.g.
    
    "Starting DB backed MetaStore Server in Secure Mode"
    
    "Starting DB backed MetaStore Server"
    
    "Starting DB backed MetaStore Server with SetUGI enabled"
     



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
<https://reviews.apache.org/r/2975/#comment9001>

    If the call to set_ugi() fails, is logging a message and continuing really the right behavior?
Why not just fail outright?
    
    Also, if you think that continuing is the correct behavior, then I think the description
of metastore.execute.setugi should be updated to explain that this is a best effort approach,
and it's possible that your setting will not be honored.



trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
<https://reviews.apache.org/r/2975/#comment8999>

    There's a TAB character here and on line 285. Please remove.



trunk/shims/src/common/java/org/apache/hadoop/hive/shims/HadoopShims.java
<https://reviews.apache.org/r/2975/#comment9013>

    Add a newline.


- Carl


On 2011-12-17 02:42:36, Ashutosh Chauhan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2975/
> -----------------------------------------------------------
> 
> (Updated 2011-12-17 02:42:36)
> 
> 
> Review request for hive.
> 
> 
> Summary
> -------
> 
> Pass user identity in metastore connection in unsecure mode
> 
> 
> This addresses bug HIVE-2616.
>     https://issues.apache.org/jira/browse/HIVE-2616
> 
> 
> Diffs
> -----
> 
>   trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1215380 
>   trunk/conf/hive-default.xml.template 1215380 
>   trunk/metastore/if/hive_metastore.thrift 1215380 
>   trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1215380 
>   trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1215380 
>   trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp 1215380

>   trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java
1215380 
>   trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php 1215380

>   trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote 1215380

>   trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py 1215380

>   trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1215380 
>   trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 1215380

>   trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
1215380 
>   trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java PRE-CREATION

>   trunk/shims/ivy.xml 1215380 
>   trunk/shims/src/0.20/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 1215380 
>   trunk/shims/src/0.20S/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java 1215380

>   trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java
1215380 
>   trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/client/TUGIAssumingTransport.java
PRE-CREATION 
>   trunk/shims/src/common/java/org/apache/hadoop/hive/shims/HadoopShims.java 1215380 
>   trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/TFilterTransport.java PRE-CREATION

>   trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/2975/diff
> 
> 
> Testing
> -------
> 
> All the tests in metastore dir passes. Manually tested that file on hdfs is owned by
user running the client and not by user running metastore server.
> 
> 
> Thanks,
> 
> Ashutosh
> 
>


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