mneethiraj commented on code in PR #289: URL: https://github.com/apache/atlas/pull/289#discussion_r1955145602
########## addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseBridge.java: ########## @@ -633,11 +631,11 @@ private AtlasEntityWithExtInfo updateEntityInAtlas(AtlasEntityWithExtInfo entit LOG.info("Updated {} entity: name={}, guid={} ", ret.getEntity().getTypeName(), ret.getEntity().getAttribute(ATTRIBUTE_QUALIFIED_NAME), ret.getEntity().getGuid()); } else { - LOG.info("Entity: name={} ", entity.toString() + " not updated as it is unchanged from what is in Atlas" ); + LOG.info("Entity: name={} ", entity.toString() + " not updated as it is unchanged from what is in Atlas"); Review Comment: Update the log message to avoid string concat: ``` LOG.info("Entity: name={} not updated as it is unchanged from what is in Atlas", entity) ``` Please review and update other such log messages as well. ########## addons/hbase-bridge-shim/src/main/java/org/apache/atlas/hbase/hook/HBaseAtlasCoprocessor.java: ########## @@ -38,34 +37,33 @@ import java.io.IOException; import java.util.Optional; - public class HBaseAtlasCoprocessor implements MasterCoprocessor, MasterObserver, RegionObserver, RegionServerObserver { public static final Log LOG = LogFactory.getLog(HBaseAtlasCoprocessor.class); private static final String ATLAS_PLUGIN_TYPE = "hbase"; private static final String ATLAS_HBASE_HOOK_IMPL_CLASSNAME = "org.apache.atlas.hbase.hook.HBaseAtlasCoprocessor"; - private AtlasPluginClassLoader atlasPluginClassLoader = null; - private Object impl = null; - private MasterObserver implMasterObserver = null; - private RegionObserver implRegionObserver = null; - private RegionServerObserver implRegionServerObserver = null; - private MasterCoprocessor implMasterCoprocessor = null; + private AtlasPluginClassLoader atlasPluginClassLoader; + private Object impl; + private MasterObserver implMasterObserver; + private RegionObserver implRegionObserver; + private RegionServerObserver implRegionServerObserver; + private MasterCoprocessor implMasterCoprocessor; public HBaseAtlasCoprocessor() { - if(LOG.isDebugEnabled()) { + if (LOG.isDebugEnabled()) { Review Comment: Please remove `LOG.isDebugEnabled()` checks as use of parameterized log statement avoids the overhead of stringt concat used earlier. ########## addons/hbase-bridge/src/main/java/org/apache/atlas/hbase/bridge/HBaseAtlasHook.java: ########## @@ -138,11 +137,10 @@ public static HBaseAtlasHook getInstance() { if (ret == null) { try { synchronized (HBaseAtlasHook.class) { - ret = me; - - if (ret == null) { - me = ret = new HBaseAtlasHook(); + if (me == null) { Review Comment: replace block 140 - 143 with the following: ``` ret = me; if (ret == null) { ret = new HBaseAtlasHook(); me = ret; } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@atlas.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org