> On May 28, 2019, 10:58 a.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org/apache/atlas/impala/hook/events/CreateImpalaProcess.java > > Line 114 (original), 114 (patched) > > <https://reviews.apache.org/r/70708/diff/1/?file=2146554#file2146554line114> > > > > surround with LOG.isDebugEnabled(); same for line #121 > > Na Li wrote: > see comment at > https://stackoverflow.com/questions/105852/conditional-logging-with-minimal-cyclomatic-complexity/105908#105908 > and > `https://stackoverflow.com/questions/963492/in-log4j-does-checking-isdebugenabled-before-logging-improve-performance` > > `Better yet is to use a more up-to-date logging framework where the log > statements take a format specification and a list of arguments to be > substituted by the logger—but "lazily," only if the logger is enabled.` > that is what I did. The overhead only happens if debug logging is > enabled. So I don't need to check `LOG.isDebugEnabled()`
this is to keep it consistent with other Log.debug statements. - Sarath ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70708/#review215546 ----------------------------------------------------------- On May 28, 2019, 2:19 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70708/ > ----------------------------------------------------------- > > (Updated May 28, 2019, 2:19 p.m.) > > > Review request for atlas, Aadarsh Jajodia, Madhan Neethiraj, and Sarath > Subramanian. > > > Repository: atlas > > > Description > ------- > > ATLAS-3133 adds a new feature to track metadata for different executions of > the same process in Atlas. Need to add this in Impala integration > > > Diffs > ----- > > > addons/impala-bridge/src/main/java/org/apache/atlas/impala/ImpalaLineageTool.java > 7c9abc8 > > addons/impala-bridge/src/main/java/org/apache/atlas/impala/hook/AtlasImpalaHookContext.java > 88faace > > addons/impala-bridge/src/main/java/org/apache/atlas/impala/hook/ImpalaIdentifierParser.java > b9d6cbb > > addons/impala-bridge/src/main/java/org/apache/atlas/impala/hook/ImpalaLineageHook.java > 232a569 > > addons/impala-bridge/src/main/java/org/apache/atlas/impala/hook/events/BaseImpalaEvent.java > c7604ba > > addons/impala-bridge/src/main/java/org/apache/atlas/impala/hook/events/CreateImpalaProcess.java > 3071576 > > addons/impala-bridge/src/test/java/org/apache/atlas/impala/ImpalaLineageITBase.java > 0138d88 > > addons/impala-bridge/src/test/java/org/apache/atlas/impala/ImpalaLineageToolIT.java > 033a518 > > addons/impala-bridge/src/test/java/org/apache/atlas/impala/hook/ImpalaLineageHookIT.java > 86801e3 > > addons/impala-bridge/src/test/resources/impalaMultipleInsertIntoAsSelect1.json > PRE-CREATION > > addons/impala-bridge/src/test/resources/impalaMultipleInsertIntoAsSelect2.json > PRE-CREATION > > > Diff: https://reviews.apache.org/r/70708/diff/2/ > > > Testing > ------- > > Integration tests > > > Thanks, > > Na Li > >
