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


Fix it, then Ship it!





intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java
Line 45 (original), 54 (patched)
<https://reviews.apache.org/r/71736/#comment306527>

    nit: revert whitespace changes from line 54-60



intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java
Lines 97 (patched)
<https://reviews.apache.org/r/71736/#comment306525>

    nit: unused method; consider removing it.



intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java
Lines 515 (patched)
<https://reviews.apache.org/r/71736/#comment306524>

    consider creating a constant for "__CLASSIFICATION_ROOT" in 
AtlasClassificationType
    
    private static final String CLASSIFICATION_ROOT_NAME = 
"__CLASSIFICATION_ROOT";



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 633 (patched)
<https://reviews.apache.org/r/71736/#comment306529>

    SUPER_TYPES_PROPERTY_KEY, TRAIT_NAMES_PROPERTY_KEY and 
PROPAGATED_TRAIT_NAMES_PROPERTY_KEY are array attributes not indexed in solr. 
Do we need to include them in system attributes (since they cannot be index 
searched)?



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 648 (patched)
<https://reviews.apache.org/r/71736/#comment306528>

    create static constant for "__ENTITY_ROOT"



intg/src/main/java/org/apache/atlas/type/AtlasStructType.java
Lines 937 (patched)
<https://reviews.apache.org/r/71736/#comment306530>

    consider refactoring if condition check to a method:
    
    if (isRootType(structDef)) { ... }
    
    private static boolean isRootType(AtlasStructDef structDef) {
            return StringUtils.equals(structDef.getName(), 
AtlasEntityType.ENTITY_ROOT.getTypeName()) ||
                    StringUtils.equals(structDef.getName(), 
AtlasClassificationType.CLASSIFICATION_ROOT.getTypeName());
        }
        
    same for line #948



intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java
Lines 63 (patched)
<https://reviews.apache.org/r/71736/#comment306531>

    refactor line 63-64 and 74-75 to a mthod: resolveReferencesForRootTypes().
    
    define getters in AtlasEntityType and AtlasClassificationType
    
    resolveReferencesForRootTypes() {
      AtlasEntityType.getEntityRoot().resolveReferences(this);
      AtlasClassificationType.getClassificationRoot().resolveReferences(this);
    }



repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java
Lines 108 (patched)
<https://reviews.apache.org/r/71736/#comment306532>

    consider refactoring if condition to method:
    
    if (!entityRootType()) {...}



repository/src/main/java/org/apache/atlas/discovery/SearchContext.java
Line 59 (original), 61 (patched)
<https://reviews.apache.org/r/71736/#comment306533>

    avoid unnecessary whitespace/newline changes 
    
    line 61-75
    line 83-90



repository/src/main/java/org/apache/atlas/discovery/SearchContext.java
Lines 80 (patched)
<https://reviews.apache.org/r/71736/#comment306534>

    AtlasEntityType.ENTITY_ROOT => AtlasEntityType.getEntityRoot();



repository/src/main/java/org/apache/atlas/discovery/SearchContext.java
Lines 315 (patched)
<https://reviews.apache.org/r/71736/#comment306535>

    entityName => entityTypeName
    
    consider refactoring method with a single return:
    
    return StringUtils.equals(entityName, ALL_ENTITY_TYPES) ? 
MATCH_ALL_ENTITY_TYPES : typeRegistry.getEntityTypeByName(entityName);


- Sarath Subramanian


On Nov. 18, 2019, 2:30 p.m., Le Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71736/
> -----------------------------------------------------------
> 
> (Updated Nov. 18, 2019, 2:30 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Aadarsh Jajodia, Sridhar K, Madhan 
> Neethiraj, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3482
>     https://issues.apache.org/jira/browse/ATLAS-3482
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Introduce ALL_ENTITY_TYPE to support search on system attributes across all 
> entity types. System attributes will be passed in as normal entity attributes.
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java 
> 8f0e5912d 
>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 
> e10965b87 
>   intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java 
> 417194202 
>   intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java 884447f81 
>   intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e8bf7f9eb 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 8b4fd1c3b 
>   intg/src/main/java/org/apache/atlas/type/Constants.java PRE-CREATION 
>   intg/src/test/java/org/apache/atlas/TestUtilsV2.java 530d5cda4 
>   
> intg/src/test/java/org/apache/atlas/entitytransform/TransformationHandlerTest.java
>  d3f4b57ad 
>   intg/src/test/java/org/apache/atlas/model/ModelTestUtil.java 5df952546 
>   intg/src/test/java/org/apache/atlas/type/TestAtlasEntityType.java 742970390 
>   intg/src/test/java/org/apache/atlas/type/TestAtlasRelationshipType.java 
> 107539598 
>   intg/src/test/java/org/apache/atlas/type/TestAtlasTypeRegistry.java 
> 476bc3300 
>   
> repository/src/main/java/org/apache/atlas/discovery/ClassificationSearchProcessor.java
>  672f38132 
>   
> repository/src/main/java/org/apache/atlas/discovery/EntitySearchProcessor.java
>  03eb92bcc 
>   
> repository/src/main/java/org/apache/atlas/discovery/FullTextSearchProcessor.java
>  152ade8d4 
>   repository/src/main/java/org/apache/atlas/discovery/SearchContext.java 
> 7ad32bdb9 
>   repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 
> b56d8e83a 
>   webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java 825cda30b 
>   webapp/src/test/java/org/apache/atlas/web/adapters/TestEntitiesREST.java 
> cd8f8981c 
> 
> 
> Diff: https://reviews.apache.org/r/71736/diff/4/
> 
> 
> Testing
> -------
> 
> - Added new unit tests, passed.
> - pre-commit job succeeds. 
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/1540/
> 
> 
> Thanks,
> 
> Le Ma
> 
>

Reply via email to