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




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

    Consider simplifying with the following alternate:
    
    StringBuilder token        = new StringBuilder();
    bool          isInAttrName = false;
    
    for (int i = 0; i < template.length(); i++) {
      char c = template.charAt(i);
    
      switch (c) {
        case DYN_ATTRIBUTE_OPEN_DELIM: {
          isInAttrName = true;
    
          if (token.length() > 0) {
            ret.add(new ConstantToken(token.toString());
    
            token.setLength(0);
          }
        }
        break;
    
        case DYN_ATTRIBUTE_CLOSE_DELIM:
          if (isInAttrName) {
            isInAttrName = false;
    
            if (token.length() > 0) {
                String attrName = token.toString();
    
                if (attrName.indexOf(DYN_ATTRIBUTE_NAME_SEPARATOR) != -1) {
                  ret.add(new DependentToken(token.toString()));
                } else {
                  ret.add(new AttributeToken(token.toString());
                }
    
                token.setLength(0);
            }
          } else {
            token.append(c);
          }
        break;
        
        default:
          token.append(c);
        break;
      }
    }



intg/src/main/java/org/apache/atlas/type/AttributeToken.java
Lines 25 (patched)
<https://reviews.apache.org/r/70913/#comment303758>

    Consider marking as 'final' as this member is not updated after 
initialization.
    
    Same for members of ConstantToken and DependentToken.



intg/src/main/java/org/apache/atlas/type/AttributeToken.java
Lines 35 (patched)
<https://reviews.apache.org/r/70913/#comment303756>

    UNKNOWN_ATTRIBUTE doesn't seem right error here. The attribute value is not 
prsent in the entity. I suggest to return null from here and have the caller 
deal with the desired action.



intg/src/main/java/org/apache/atlas/type/DependentToken.java
Lines 37 (patched)
<https://reviews.apache.org/r/70913/#comment303757>

    Is the last entry the attrName? For example, consider  token 
'{table.db.name}' for hive_column.qualifiedName. 'table' would be the attribute 
name, which is the first entry. Please review.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityGraphDiscoveryV2.java
Lines 132 (patched)
<https://reviews.apache.org/r/70913/#comment303759>

    - dynamicAttributeGenerator() => processDynanicAttributes()
    - move 'private' method to end of the class, after all public methods.


- Madhan Neethiraj


On July 9, 2019, 11:55 p.m., Merryle Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70913/
> -----------------------------------------------------------
> 
> (Updated July 9, 2019, 11:55 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 
> 0fe47bdcbe84c06545c517fec3177ef7e6487a6c 
>   intg/src/main/java/org/apache/atlas/type/AttributeToken.java PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/type/ConstantToken.java PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/type/DependentToken.java PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/type/TemplateToken.java PRE-CREATION 
>   intg/src/test/java/org/apache/atlas/type/TestAtlasEntityType.java 
> 3c53c02b48747515217c9327c98209a48ee84237 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityGraphDiscoveryV2.java
>  4ff4206471bb8b2b4997c00dca3b4433626ae392 
> 
> 
> Diff: https://reviews.apache.org/r/70913/diff/10/
> 
> 
> Testing
> -------
> 
> Added unit test
> 
> 
> Thanks,
> 
> Merryle Wang
> 
>

Reply via email to