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




intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java
Lines 65 (patched)
<https://reviews.apache.org/r/71736/#comment306318>

    ALL_ENTITY_TYPE_QUERY - this doesn't have to be in 'models' package; 
consider moving this out of 'intg' library, to 'repository' module.



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

    - for easier read, please have each attribute in a separate line
    - only indexed attributes are relevant for search. Consider excluding 
non-indexed attributes - like IS_PROXY_KEY, HOME_ID_KEY



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

    Consider following alternate approach, to avoid instantiating 
system-attributes in each AtlasEntityType/AtlasClassificationType instance:
      class AtlasTypeRegistry {
        private final AtlasEntityType         baseEntityType         = 
initBaseEntityType();
        private final AtlasClassificationType baseClassificationType = 
initBaseClassificationType();
    
        AtlasEntityType getBaseEntityType() { return baseEntityType; }
    
        AtlasClassificationType getBaseClassificationType() { return 
baseClassificationType; }
    
        private static AtlasEntityType initBaseEntityType() {
          List<AtlasAttributeDef> attributeDefs = new ArrayList<>() {{
            add(new AtlasAttributeDef(ENTITY_TYPE_PROPERTY_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, false, true)));
            add(new AtlasAttributeDef(GUID_PROPERTY_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, true, true)));
            add(new AtlasAttributeDef(TRAIT_NAMES_PROPERTY_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, AtlasAttributeDef.Cardinality.SET, false, 
true)));
            add(new AtlasAttributeDef(PROPAGATED_TRAIT_NAMES_PROPERTY_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, AtlasAttributeDef.Cardinality.LIST, false, 
true)));
            add(new AtlasAttributeDef(HISTORICAL_GUID_PROPERTY_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, false, true)));
            add(new AtlasAttributeDef(CLASSIFICATION_TEXT_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, false, true)));
            add(new AtlasAttributeDef(CLASSIFICATION_NAMES_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, false, true)));
            add(new AtlasAttributeDef(PROPAGATED_CLASSIFICATION_NAMES_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, false, true)));
            add(new AtlasAttributeDef(IS_INCOMPLETE_PROPERTY_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, false, true)));
            add(new AtlasAttributeDef(LABELS_PROPERTY_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, false, true)));
            add(new AtlasAttributeDef(CUSTOM_ATTRIBUTES_PROPERTY_KEY, 
AtlasBaseTypeDef.ATLAS_TYPE_STRING, false, true)));
          }}
          
          AtlasEntityDef entityDef = new AtlasEntityDef("", "", "", 
attributeDefs);
    
          return new AtlasEntityType(entityDef);
        }
      }
    
      class AtlasEntityType {
        private AtlasEntityType baseType = null;
    
        void resolveReferences(AtlasTypeRegistry typeRegistry) {
          baseType = typeRegistry.getBaseEntityType();
    
          ...
        }
    
        public boolean isSystemAttribute(String attributeName) {
          return baseType != null && baseType.hasAttribute(attributeName);
        }
    
        public AtlasAttribute getSystemAttribute(String attributeName) {
          return baseType != null ? baseType.getAttribute(attributeName) : null;
        }
      }
    
      class AtlasClassificationType {
        private AtlasClassificationType baseType = null;
    
        void resolveReferences(AtlasTypeRegistry typeRegistry) {
          baseType = typeRegistry.getBaseClassificationType();
    
          ...
        }
    
        public boolean isSystemAttribute(String attributeName) {
          return baseType != null && baseType.hasAttribute(attributeName);
        }
    
        public AtlasAttribute getSystemAttribute(String attributeName) {
          return baseType != null ? baseType.getAttribute(attributeName) : null;
        }
      }



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

    AtlasAttribute instantiation here will set qualifiedName and 
vertextPropertyName to:
      this.typeName + "." + sysAttributeName" - like hive_table.__guid
      
    For system attributes, this isn't correct. The vertex property name is 
simply the value of sysAttributeName. Please review AtlasAttribute constructor 
and update it to handle system attributes for base-types (i.e. empty typename)


- Madhan Neethiraj


On Nov. 9, 2019, 1:12 a.m., Le Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71736/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2019, 1:12 a.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/AtlasBuiltInTypes.java ed1e5ded2 
>   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/Constants.java PRE-CREATION 
>   
> 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/repository/graph/GraphBackedSearchIndexer.java
>  7c551304b 
>   webapp/src/test/java/org/apache/atlas/web/adapters/TestEntitiesREST.java 
> cd8f8981c 
> 
> 
> Diff: https://reviews.apache.org/r/71736/diff/2/
> 
> 
> Testing
> -------
> 
> - Added new unit tests, passed.
> - will run pre-commit job.
> 
> 
> Thanks,
> 
> Le Ma
> 
>

Reply via email to