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




addons/models/1000-Hadoop/patches/005-hdfs_path_add_nameServiceId_attribute.json
Lines 1 (patched)
<https://reviews.apache.org/r/65647/#comment277754>

    Given migration from earlier version of Atlas to 1.0 will be via 
export/import, I think it will be better to add 'nameServiceId' attribute 
directly in hdfs_path entity-def - instead of adding via a patch.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 43 (patched)
<https://reviews.apache.org/r/65647/#comment277764>

    Caching all the URLs handled by HdfsNameServiceResolver may result in the 
map to grow very large and take up a lot of memory. Consider avoding this cache.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 56 (patched)
<https://reviews.apache.org/r/65647/#comment277766>

    Consider using int for paramter 'port'.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 98 (patched)
<https://reviews.apache.org/r/65647/#comment277755>

    This would replace all occurances of "uri.getAuthority()" in the URL with 
nsId - which may not be desired. Only the first instance should be replaced:
    
    hdfs://myhost/d1/d2/myhost.txt ==> hdfs://ns1/d1/d2/ns1.txt.
    
    Please review and update.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 230 (patched)
<https://reviews.apache.org/r/65647/#comment277760>

    Why not directly add these constants in outer class? Just as 
'NS_ID_FOR_PATH', 'PATH_WITH_NSID' were added above.



common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java
Lines 231 (patched)
<https://reviews.apache.org/r/65647/#comment277757>

    I think the default port can be configured in HDFS configuration. Instead 
of hardcoding "8020" here, read default port from HDFS configuration.


- Madhan Neethiraj


On Feb. 14, 2018, 6:36 p.m., Apoorv Naik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65647/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2018, 6:36 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, and Sarath 
> Subramanian.
> 
> 
> Bugs: ATLAS-2444
>     https://issues.apache.org/jira/browse/ATLAS-2444
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> See ATLAS-2444
> 
> 
> Diffs
> -----
> 
>   
> addons/hive-bridge/src/main/java/org/apache/atlas/hive/bridge/HiveMetaStoreBridge.java
>  7cca48f11 
>   
> addons/models/1000-Hadoop/patches/005-hdfs_path_add_nameServiceId_attribute.json
>  PRE-CREATION 
>   
> addons/storm-bridge/src/main/java/org/apache/atlas/storm/hook/StormAtlasHook.java
>  57fc7a1fd 
>   common/pom.xml a7ab6a312 
>   common/src/main/java/org/apache/atlas/utils/HdfsNameServiceResolver.java 
> PRE-CREATION 
>   
> common/src/test/java/org/apache/atlas/utils/HdfsNameServiceResolverTest.java 
> PRE-CREATION 
>   common/src/test/resources/atlas-log4j.xml PRE-CREATION 
>   common/src/test/resources/hdfs-site.xml PRE-CREATION 
>   pom.xml c15e0dad7 
> 
> 
> Diff: https://reviews.apache.org/r/65647/diff/2/
> 
> 
> Testing
> -------
> 
> mvn clean package executes successfully.
> 
> The modified war was tested on a multi-node cluster with one nameServiceId 
> and 2 NameNodes, the required information was captured successfully in the 
> entity when an external table was created from beeline (hive context)
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Reply via email to