----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59821/#review185093 -----------------------------------------------------------
addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java Lines 76 (patched) <https://reviews.apache.org/r/59821/#comment261314> Looks like constants in line #76 - #87 are for attribute names. It will be helpful to prefix these names with "ATTR_". Perhaps line #74 as well. addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java Lines 132 (patched) <https://reviews.apache.org/r/59821/#comment261315> It will be useful to print exception as well: LOG.info("Interrupt received in shutdown.", ie); addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java Lines 148 (patched) <https://reviews.apache.org/r/59821/#comment261316> "try {" at line #148 can move inside 'if' block at line #149. addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java Lines 183 (patched) <https://reviews.apache.org/r/59821/#comment261317> 'operation' is already part of 'hbaseOperationContext'. It will be good to not pass this as a separate argument. Same for line #190 and #197 as well. addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java Lines 205 (patched) <https://reviews.apache.org/r/59821/#comment261318> nameSpaceRef is sent as null from the caller at line #183. It this argument is not necessary, please remove. addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java Lines 272 (patched) <https://reviews.apache.org/r/59821/#comment261319> - namespaceReference and tableReference are sent as null from the caller at line #190. It these arguments are not necessary, please remove. - this method doesn't seem to be used outside this class. In that case, consider marking this a private method. addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java Lines 332 (patched) <https://reviews.apache.org/r/59821/#comment261321> - this method doesn't seem to be used outside this class. In that case, consider marking this a private method. addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java Lines 350 (patched) <https://reviews.apache.org/r/59821/#comment261320> - namespaceReference and tableReference are sent as null from the caller at line #197. It these arguments are not necessary, please remove. - this method doesn't seem to be used outside this class. In that case, consider marking this a private method. addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java Lines 435 (patched) <https://reviews.apache.org/r/59821/#comment261322> to be consistent, avoid space after "(". Also, I see other places that miss a space after a ",". Please review and add a space after a ",". addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java Lines 854 (patched) <https://reviews.apache.org/r/59821/#comment261324> 'user' could be null - from line #838. Please update here to handle this case. - Madhan Neethiraj On Sept. 10, 2017, 7:04 a.m., Ramesh Mani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59821/ > ----------------------------------------------------------- > > (Updated Sept. 10, 2017, 7:04 a.m.) > > > Review request for atlas and Madhan Neethiraj. > > > Bugs: ATLAS-1805 > https://issues.apache.org/jira/browse/ATLAS-1805 > > > Repository: atlas > > > Description > ------- > > ATLAS-1805: Provide an Atlas hook to send Hbase Namespace/Table/column family > metadata to Atlas > > > Diffs > ----- > > addons/hbase-bridge-shim/pom.xml PRE-CREATION > > addons/hbase-bridge-shim/src/main/java/org/apache/atlas/hbase/hook/HBaseAtlasCoprocessor.java > PRE-CREATION > addons/hbase-bridge/pom.xml PRE-CREATION > > addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java > PRE-CREATION > > addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/hook/HBaseAtlasCoprocessor.java > PRE-CREATION > > addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/hook/HBaseAtlasCoprocessorBase.java > PRE-CREATION > > addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/model/HBaseDataTypes.java > PRE-CREATION > > addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/model/HBaseOperationContext.java > PRE-CREATION > addons/hbase-bridge/src/main/resources/atlas-hbase-import-log4j.xml > PRE-CREATION > > addons/hbase-bridge/src/test/java/org/apache/atlas/hbase/HBaseAtlasHookTest.java > PRE-CREATION > addons/models/1060-hbase_model.json 3e46e06 > addons/models/patches/001-hbase_table_column_family_add_attribute.json > PRE-CREATION > distro/src/main/assemblies/standalone-package.xml 215cb23 > pom.xml 157122a > > > Diff: https://reviews.apache.org/r/59821/diff/4/ > > > Testing > ------- > > * Review comments fixed with 2 exceptions whcih I have commented on > * Added an IT test > * Testing done in LOCAL VM > > > Thanks, > > Ramesh Mani > >
