-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70512/#review215151
-----------------------------------------------------------




addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java
Lines 82 (patched)
<https://reviews.apache.org/r/70512/#comment301662>

    I might be missing some thing here. Why are we deleting the wal file? Did 
we have some change in the way the lineage files are generated? Normally, we 
used to have the lineage file continously appeneded with new lineage records. 
Isn't that the case any more?



addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java
Lines 82 (patched)
<https://reviews.apache.org/r/70512/#comment301663>

    I might be missing some thing. Why are we deleting the wal file? Did we 
have some change in the way the lineage files are generated? Normally, we used 
to have the lineage file continously appeneded with new lineage records. Isn't 
that the case any more?



addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java
Lines 124 (patched)
<https://reviews.apache.org/r/70512/#comment301653>

    This might be what is the scenario. I suggest that wqe refer the script 
name in the script and class name in the class. Can we please fix this?



addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java
Lines 134 (patched)
<https://reviews.apache.org/r/70512/#comment301654>

    Can we call it, getActiveLineageFiles() instead?



addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java
Lines 138 (patched)
<https://reviews.apache.org/r/70512/#comment301655>

    Do we want prefix to be user configurable? This makes the user to choose 
the name after using a prefix for some time and changing it to some thing 
different. Now, you have bunch of old files lying around. :-).



addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java
Lines 154 (patched)
<https://reviews.apache.org/r/70512/#comment301657>

    Can you create a CONSTANT and reuse it?



addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java
Lines 158 (patched)
<https://reviews.apache.org/r/70512/#comment301656>

    Do we need this variable?
    
    Why not return the value directly?



addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java
Lines 181 (patched)
<https://reviews.apache.org/r/70512/#comment301658>

    importHImapala ==> importImpala
    
    name ==> fileName.



addons/impala-bridge/src/main/java/org.apache.atlas.impala/ImpalaLineageTool.java
Lines 189 (patched)
<https://reviews.apache.org/r/70512/#comment301659>

    Can we move these three lines into one method?
    
    On a side note, why do we even maintain a wal file, when you delete the wal 
file immediately after returning from this method?


- Sridhar K


On May 8, 2019, 9:36 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70512/
> -----------------------------------------------------------
> 
> (Updated May 8, 2019, 9:36 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 ae4dfdc 
> 
> 
> Diff: https://reviews.apache.org/r/70512/diff/16/
> 
> 
> 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
> 
>

Reply via email to