----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70512/#review215106 -----------------------------------------------------------
addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java Lines 48 (patched) <https://reviews.apache.org/r/70512/#comment301556> we don't need 'args' to be stored as class variable. please review. addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java Lines 52 (patched) <https://reviews.apache.org/r/70512/#comment301555> consider intializing values for directoryName, prefix in constructor instead of getCurrentFiles() addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java Lines 57 (patched) <https://reviews.apache.org/r/70512/#comment301554> why 'args' need to be passed in method, when its already initialized in constructor (line #53)? addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java Lines 127 (patched) <https://reviews.apache.org/r/70512/#comment301551> BasicParser() is deprecated; use DefaultParser() instead. addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java Lines 155 (patched) <https://reviews.apache.org/r/70512/#comment301557> this method doesn't do anything to send lineage to kafka; consider renaming method to processImpalaLineageHook() addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java Lines 211 (patched) <https://reviews.apache.org/r/70512/#comment301559> update logger message - Error sending impala lineage records to ImpalaHook addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/AtlasImpalaHookContext.java Lines 159 (patched) <https://reviews.apache.org/r/70512/#comment301569> why table name needs to be checked for correctness? lineage query is written by impala after successfull query execution. Impala would have already validated the table name correctness. We don't need to re-validate here again. Consider removing ImpalaIdentifierParser class as well. addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/ImpalaOperationParser.java Lines 31 (patched) <https://reviews.apache.org/r/70512/#comment301561> consider renaming method to getImpalaOperationType() addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/events/BaseImpalaEvent.java Lines 78 (patched) <https://reviews.apache.org/r/70512/#comment301566> depenendencyType => dependencyType addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/events/BaseImpalaEvent.java Lines 79 (patched) <https://reviews.apache.org/r/70512/#comment301567> unused variable - ATTRIBUTE_EXPRESSION addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/events/CreateImpalaProcess.java Lines 21 (patched) <https://reviews.apache.org/r/70512/#comment301564> unused import; consider removing. addons/impala-bridge/src/main/resources/import-impala.sh Lines 52 (patched) <https://reviews.apache.org/r/70512/#comment301548> if we need to add hive conf in classpath, instead of using HS2 conf path get the conf similar to import-hive.sh https://github.com/apache/atlas/blob/master/addons/hive-bridge/src/bin/import-hive.sh#L60 addons/impala-bridge/src/main/resources/import-impala.sh Lines 60 (patched) <https://reviews.apache.org/r/70512/#comment301549> why do we hardcode ATLAS lib path here? addons/models/1000-Hadoop/1090-impala_model.json Lines 2 (patched) <https://reviews.apache.org/r/70512/#comment301545> Add empty "enumDefs" and "structDefs" as well to avoid deserialization issues. addons/models/1000-Hadoop/1090-impala_model.json Lines 104 (patched) <https://reviews.apache.org/r/70512/#comment301544> depenendencyType => dependencyType addons/models/1000-Hadoop/1090-impala_model.json Lines 194 (patched) <https://reviews.apache.org/r/70512/#comment301546> remove "relationshipLabel" entry - This is added only for legacy models. Also remove "isLegacyAttribute": true in line #200 - Sarath Subramanian On May 4, 2019, 2:23 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70512/ > ----------------------------------------------------------- > > (Updated May 4, 2019, 2:23 p.m.) > > > Review request for atlas, Ashutosh Mestry, Aadarsh Jajodia, madhan, Sarath > Subramanian, and Xinran Tinney. > > > Repository: atlas > > > Description > ------- > > Impala generates lineage records for its commands. This new feature will read > Impala lineage file, convert the lineage record to Atlas entities and send > them to Atlas. In this way, Atlas can get lineage of Impala operation. > > The metadata referred in the lineage are captured in Hive Metastore hook and > sent to Atlas. This work is done in ATLAS-3148 > > This jira only supports the Impala command "create view". Following jira will > add support for more Impala commands. > > > Diffs > ----- > > addons/impala-bridge/pom.xml PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/AtlasImpalaHookContext.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/ImpalaIdentifierParser.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/ImpalaLineageHook.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/ImpalaOperationParser.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/events/BaseImpalaEvent.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/events/CreateImpalaProcess.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/model/IImpalaLineageHook.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/model/ImpalaDataType.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/model/ImpalaDependencyType.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/model/ImpalaNode.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/model/ImpalaOperationType.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/model/ImpalaQuery.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/model/ImpalaVertexType.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/model/LineageEdge.java > PRE-CREATION > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/model/LineageVertex.java > PRE-CREATION > addons/impala-bridge/src/main/resources/atlas-log4j.xml PRE-CREATION > addons/impala-bridge/src/main/resources/import-impala.sh PRE-CREATION > > addons/impala-bridge/src/test/java/org/apache/atlas/impala/ImpalaLineageITBase.java > PRE-CREATION > > addons/impala-bridge/src/test/java/org/apache/atlas/impala/ImpalaLineageToolIT.java > PRE-CREATION > > addons/impala-bridge/src/test/java/org/apache/atlas/impala/hook/ImpalaLineageHookIT.java > PRE-CREATION > addons/impala-bridge/src/test/resources/atlas-application.properties > PRE-CREATION > addons/impala-bridge/src/test/resources/atlas-log4j.xml PRE-CREATION > addons/impala-bridge/src/test/resources/hive-site.xml PRE-CREATION > addons/impala-bridge/src/test/resources/impala1.json PRE-CREATION > addons/impala-bridge/src/test/resources/impala2.json PRE-CREATION > addons/impala-bridge/src/test/resources/impala3.json PRE-CREATION > addons/impala-bridge/src/test/resources/users-credentials.properties > PRE-CREATION > addons/models/1000-Hadoop/1090-impala_model.json PRE-CREATION > pom.xml 7de5d31 > > > Diff: https://reviews.apache.org/r/70512/diff/14/ > > > Testing > ------- > > Run the tool in real cluster that has Atlas server with Impala lineage file > as input for creating view. The Atlas UI displays hive_lineage lineage and > hive_column_lineage. > Add new integration tests and they pass > > > Thanks, > > Na Li > >
