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




addons/models/1000-Hadoop/1030-hive_model.json
Lines 451 (patched)
<https://reviews.apache.org/r/70913/#comment303224>

    "outputs" is a collection, hence referencing attribute 'name' would be 
ambiguous. I suggest to not support collection type attributes in dynAttribute 
expressions.



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 171 (patched)
<https://reviews.apache.org/r/70913/#comment303225>

    relationshipAttributes is finalized only in resolveReferencesPhase3(). 
Given populateDynFlagsInfo() might be referring to relationship attributes, 
lines #170 - #173 should be moved to resolveReferencesPhase3().



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 675 (patched)
<https://reviews.apache.org/r/70913/#comment303227>

    clear contents of dynAttributes, dynEvalTriggerAttributes before populating 
them here.



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 683 (patched)
<https://reviews.apache.org/r/70913/#comment303230>

    Please consider adding a comment here, if and else case here i.e. 
presence/absence of DYN_ATTRIBUTE_NAME_SEPARATOR in exp.



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 690 (patched)
<https://reviews.apache.org/r/70913/#comment303228>

    Instead of checking for null here, update the block in #677 to avoid adding 
null entries in dynAttributes.



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 696 (patched)
<https://reviews.apache.org/r/70913/#comment303229>

    Instead of checking for null here, update the block in #684 to avoid adding 
null entries in dynAttributes.



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 708 (patched)
<https://reviews.apache.org/r/70913/#comment303226>

    1. consider folllwing renames for better readabilitry"
       attributeName => key
       key           => attributeName
    
    2. Consider updating this method to return parsed-map and have it assigned 
to this.parsedTemplates:
    
       private Map<String, List<String>> parseDynAttributeTemplates() {
         Map<String, List<String>> ret = new HashMap<>();
         
         ... // parse and populate ret
         
         return Collections.unmodifiableMap(ret);
       }


- Madhan Neethiraj


On June 26, 2019, 8:14 p.m., Merryle Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70913/
> -----------------------------------------------------------
> 
> (Updated June 26, 2019, 8:14 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Aadarsh Jajodia, Sridhar K, Le Ma, 
> Madhan Neethiraj, and Sarath Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-3286: Populated dynamic attribute flags for AtlasAttributes
> 
> 
> Diffs
> -----
> 
>   addons/models/1000-Hadoop/1030-hive_model.json 
> 8901aa4aa86fb2802a9e9b1e65c2ff8aad8855ad 
>   intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java 
> d9ae9e36773146fad652a1e28fc8822bae5c8557 
>   intg/src/main/java/org/apache/atlas/type/AtlasStructType.java 
> 254eee4c9d18ab87a8862ce479fc2274227c7c29 
>   intg/src/test/java/org/apache/atlas/type/TestAtlasEntityType.java 
> 3c53c02b48747515217c9327c98209a48ee84237 
> 
> 
> Diff: https://reviews.apache.org/r/70913/diff/4/
> 
> 
> Testing
> -------
> 
> Planning to add unit tests
> 
> 
> Thanks,
> 
> Merryle Wang
> 
>

Reply via email to