atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sarath Subramanian <sar...@apache.org>
Subject Re: Review Request 71649: ATLAS-3486 Define Data Models For Namespaces and Namespace Attributes in Atlas and also the corresponding type registry changes for the same
Date Thu, 21 Nov 2019 22:10:26 GMT

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



I don't see namespace attribuites indexed. This should be indexed. 

Check GraphBackSearchIndexer.addIndexForType() method


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

    removed unused constructors in line 174, 179, 193 and 203



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

    'maxStrLength' and 'validPattern' are string specific attribute options. Consider adding
a enum - namespaceAttributeOptions {MAX_STRING_LENGTH, VALID_PATTERN} and use 'options' map
in AtlasAttributeDef to set the namespace attribute options. This will be easier to extend
for any int, date options as well.



intg/src/main/java/org/apache/atlas/store/AtlasTypeDefStore.java
Lines 83 (patched)
<https://reviews.apache.org/r/71649/#comment306617>

    add REST endpoints in TypesREST for:
    
    /namespacedef/name/{name}
    /namespacedef/guid/{guid}



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

    nit: remove unused imports in line 22,42



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

    validateType() checks for invalid types only for structDef attruibutes, add check in validateType()
to check for namespace attributes as well.



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

    is this line needed? please review.



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

    "update classification-def " => "update namespace-def "



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

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



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

    AtlasBaseException is never thrown



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

    'if' condition in line 349 and 353 is duplicate. please review.



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

    check for null as well for 'applicableEntityTypes'. => CollectionUtils.isNotEmpty(attributeDef.getApplicableEntityTypes())



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

    => CollectionUtils.isNotEmpty(attributeDef.getApplicableEntityTypes())



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

    refactor to:
    
    if (!currAttrNames.containsAll(attrNames)) {
     //throw exception
    }



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

    check for null attributeDef.getApplicableEntityTypes() as well ?



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

    => if(!updatedApplicableEntityTypes.containsAll(existingAttribute.getApplicableEntityTypes()))
{
    // throw exception
    }



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

    set using encoded property key here:
    
    AtlasGraphUtilsV2.encodePropertyKey(propertyKey);



repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java
Line 694 (original), 694 (patched)
<https://reviews.apache.org/r/71649/#comment306616>

    contains only whitespace changes. revert changes to this file


- Sarath Subramanian


On Nov. 20, 2019, 7:03 p.m., Aadarsh Jajodia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71649/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2019, 7:03 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/AtlasErrorCode.java 7a2aae2e9ae8174c5309164f3e41c940cbf3ddf8

>   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 bb7ead0f9f8bab3094eb82e9e286dd58e8a6e3de

>   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 557ef74a95c2a939b4b89cd1db8fa4c73d52dd51

>   intg/src/main/java/org/apache/atlas/type/AtlasNamespaceType.java PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java b071dc9d664cee9e1ffc54726ffbf15f4f602d30

>   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 ff79994c519702e90b2e478d00cae0008889f956

>   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 
>   webapp/src/main/java/org/apache/atlas/examples/QuickStartV2.java 6cd0ee331b7ae24757b58e76ec47bf556106846a

> 
> 
> Diff: https://reviews.apache.org/r/71649/diff/7/
> 
> 
> Testing
> -------
> 
> Added unit tests
> 
> 
> Thanks,
> 
> Aadarsh Jajodia
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message