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




intg/src/main/java/org/apache/atlas/model/typedef/AtlasNamespaceDef.java
Lines 46 (patched)
<https://reviews.apache.org/r/71649/#comment306345>

    I suggest to use List instead  of Set, to be consistent with other 
attributeDef lists - in AtlasStructDef 
(AtlasEntityDef/AtlasClassificationDef/AtlasRelationshipDef).



intg/src/main/java/org/apache/atlas/model/typedef/AtlasTypesDef.java
Line 83 (original), 86 (patched)
<https://reviews.apache.org/r/71649/#comment306346>

    Consider adding a constructor, instead of updating existing one.



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 271 (patched)
<https://reviews.apache.org/r/71649/#comment306347>

    Consider marking populateNamespaceAttributes() method as private, as this 
is called only from this class.  Also, move this impl to later in the class, 
after all public and protected methods.



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 272 (patched)
<https://reviews.apache.org/r/71649/#comment306348>

    Instead of iteraring namespaceDef, consider iterating through 
namespaceTypes:
    
      private void populateNamespaceAttributes(AtlasTypeRegistry typeRegistry) {
        for (AtlasNamespaceType nsType : typeRegistry.getAllNamespaceTypes()) {
          for (AtlasNamespaceAttribute nsAttribute : 
namespaceType.getAttributes()) {
            if 
(nsAttribute.getApplicableEntityTypes().contains(this.getTypeName())) {
              addNamespaceAttribute(nsAttribute);
            }
          }
        }
      }
    
      private void addNamespaceAttribute(AtlasNamespaceAttribute nsAttribute) {
        String                        nsName     = 
nsAttribute.getDefinedInType().getTypeName();
        List<AtlasNamespaceAttribute> attributes = 
namespaceAttributes.get(nsName);
    
        if (attributes == null) {
          attributes = new ArrayList<>();
    
          namespaceAttributes.put(nsName, attributes);
        }
    
        attributes.add(nsAttribute);
      }



intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 272 (patched)
<https://reviews.apache.org/r/71649/#comment306349>

    Instead of iteraring namespaceDef, consider iterating through 
namespaceTypes:
    
      private void populateNamespaceAttributes(AtlasTypeRegistry typeRegistry) {
        for (AtlasNamespaceType nsType : typeRegistry.getAllNamespaceTypes()) {
          for (AtlasNamespaceAttribute nsAttribute : 
namespaceType.getAttributes()) {
            if 
(nsAttribute.getApplicableEntityTypes().contains(this.getTypeName())) {
              addNamespaceAttribute(nsAttribute);
            }
          }
        }
      }
    
      private void addNamespaceAttribute(AtlasNamespaceAttribute nsAttribute) {
        String                        nsName     = 
nsAttribute.getDefinedInType().getTypeName();
        List<AtlasNamespaceAttribute> attributes = 
namespaceAttributes.get(nsName);
    
        if (attributes == null) {
          attributes = new ArrayList<>();
    
          namespaceAttributes.put(nsName, attributes);
        }
    
        attributes.add(nsAttribute);
      }



intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java
Line 310 (original), 302 (patched)
<https://reviews.apache.org/r/71649/#comment306350>

    retain existing getTypesDef() method, to avoid breaking exising callers of 
this method. Add a new method that takes 'namespaces' parameter.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java
Lines 191 (patched)
<https://reviews.apache.org/r/71649/#comment306353>

    "struct-def" => "namespace-def"



repository/src/test/java/org/apache/atlas/repository/tagpropagation/ClassificationPropagationTest.java
Line 619 (original), 619 (patched)
<https://reviews.apache.org/r/71649/#comment306352>

    existing constructor for AtlasTypesDef should be retained; changes to this 
file can be reverted.



tools/classification-updater/src/main/java/org/apache/atlas/tools/BulkFetchAndUpdate.java
Line 524 (original), 524 (patched)
<https://reviews.apache.org/r/71649/#comment306351>

    existing constructor for AtlasTypesDef should be retained; changes to this 
file can be reverted.


- Madhan Neethiraj


On Nov. 8, 2019, 9:42 p.m., Aadarsh Jajodia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71649/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2019, 9:42 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Sridhar K, Le Ma, Madhan 
> Neethiraj, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3486
>     https://issues.apache.org/jira/browse/ATLAS-3486
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> This change is the first part of the bigger task defined as part of 
> ATLAS-3485.
> This adds the data model needed for supporting namespaces and namespace 
> attributes and also updates the type registry to include the applicable 
> namespace attributes for every entity type
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/model/TypeCategory.java 
> f06f64f450f407e3f9a0e742726ff4dd12ccc695 
>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasNamespaceDef.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java 
> e10965b870c4a300b41e93ee046b5f6d6b722728 
>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasTypesDef.java 
> 3634fdfd313639eb97b3c4698e091487b0e44a80 
>   intg/src/main/java/org/apache/atlas/store/AtlasTypeDefStore.java 
> 4ee68a936f99bb4c819b5335da2cc8bf7d539397 
>   intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java 
> 884447f81c57faf917a8d0565fc0a0c7ebbd99f0 
>   intg/src/main/java/org/apache/atlas/type/AtlasNamespaceType.java 
> PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java 
> 8b4fd1c3b9005d0a8852f2828475b4ad6a806822 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 
> 0883d54f490e22c6510e6fc0cb804b87713a7ecb 
>   intg/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java 
> dba2d88146eff314191ae6bb24ad7337b0ea10ae 
>   intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java 
> 02613b5f7250b14324ed294c22de079b74d55b08 
>   intg/src/test/java/org/apache/atlas/TestUtilsV2.java 
> 530d5cda44b7d3746f5cb8dd5a23b3c68b254cac 
>   
> intg/src/test/java/org/apache/atlas/model/typedef/TestAtlasNamespaceDef.java 
> PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java
>  2e2ab1a664171555c57560e1c0b4cbdbc20c0f6f 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasTypeDefGraphStoreV2.java
>  a5ccfb5b2055c88f596312f4033bc0034d3d165c 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java
>  51dd16b8518c9a16088547f3e95c0ef401695895 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasNamespaceDefStoreV2Test.java
>  PRE-CREATION 
>   
> repository/src/test/java/org/apache/atlas/repository/tagpropagation/ClassificationPropagationTest.java
>  6f9c05e7a7a30c678a0a376d25402c6026d0b391 
>   
> tools/classification-updater/src/main/java/org/apache/atlas/tools/BulkFetchAndUpdate.java
>  91ff89a00d779af9437ba5f6d129bd4595e5f036 
>   webapp/src/main/java/org/apache/atlas/examples/QuickStartV2.java 
> 6cd0ee331b7ae24757b58e76ec47bf556106846a 
> 
> 
> Diff: https://reviews.apache.org/r/71649/diff/3/
> 
> 
> Testing
> -------
> 
> Added unit tests
> 
> 
> Thanks,
> 
> Aadarsh Jajodia
> 
>

Reply via email to