----------------------------------------------------------- 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 > >
