> On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java > > Lines 48 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142926#file2142926line48> > > > > we don't need 'args' to be stored as class variable. please review.
removed it as class variable. Pass it in as function input > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java > > Lines 52 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142926#file2142926line52> > > > > consider intializing values for directoryName, prefix in constructor > > instead of getCurrentFiles() done > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java > > Lines 57 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142926#file2142926line57> > > > > why 'args' need to be passed in method, when its already initialized in > > constructor (line #53)? removed. > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java > > Lines 127 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142926#file2142926line127> > > > > BasicParser() is deprecated; use DefaultParser() instead. changed > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java > > Lines 155 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142926#file2142926line155> > > > > this method doesn't do anything to send lineage to kafka; consider > > renaming method to processImpalaLineageHook() done. > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java > > Lines 211 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142926#file2142926line211> > > > > update logger message - Error sending impala lineage records to > > ImpalaHook done > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/AtlasImpalaHookContext.java > > Lines 159 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142927#file2142927line159> > > > > 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. Impala output the column name in vertices. We need to derive the table name from that column name. In some vertex, the column name is not really a column name, so derived is not real table name. Like following example shows. Impala does not validate the table or column name correctness. That is why we need to do it here. "queryText":"select sum(a.tinyint_col) over (partition by a.smallint_col order by a.id),\n count(b.string_col), b.timestamp_col\nfrom functional.alltypes a join functional.alltypessmall b on (a.id = b.id)\nwhere a.year = 2010 and b.float_col > 0\ngroup by a.tinyint_col, a.smallint_col, a.id, b.string_col, b.timestamp_col, b.bigint_col\nhaving count(a.int_col) > 10\norder by b.bigint_col limit 10", "vertices":[ { "id":0, "vertexType":"COLUMN", "vertexId":"sum(a.tinyint_col) OVER(...)" }, > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/ImpalaOperationParser.java > > Lines 31 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142930#file2142930line31> > > > > consider renaming method to getImpalaOperationType() done > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/events/BaseImpalaEvent.java > > Lines 78 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142931#file2142931line78> > > > > depenendencyType => dependencyType done > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/events/BaseImpalaEvent.java > > Lines 79 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142931#file2142931line79> > > > > unused variable - ATTRIBUTE_EXPRESSION removed > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/impala-bridge/src/main/java/org.apache.atlas.impala/hook/events/CreateImpalaProcess.java > > Lines 21 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142932#file2142932line21> > > > > unused import; consider removing. removed > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/models/1000-Hadoop/1090-impala_model.json > > Lines 2 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142954#file2142954line2> > > > > Add empty "enumDefs" and "structDefs" as well to avoid deserialization > > issues. added > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/models/1000-Hadoop/1090-impala_model.json > > Lines 104 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142954#file2142954line104> > > > > depenendencyType => dependencyType done > On May 7, 2019, 11:37 p.m., Sarath Subramanian wrote: > > addons/models/1000-Hadoop/1090-impala_model.json > > Lines 194 (patched) > > <https://reviews.apache.org/r/70512/diff/14/?file=2142954#file2142954line194> > > > > remove "relationshipLabel" entry - This is added only for legacy > > models. Also remove "isLegacyAttribute": true in line #200 done - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70512/#review215106 ----------------------------------------------------------- On May 4, 2019, 9: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, 9: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 > >
