atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Radley <david_rad...@uk.ibm.com>
Subject Re: Review Request 60938: [ATLAS-1959]: Enhance relationship attributes to support different cardinality mappings
Date Wed, 19 Jul 2017 15:11:43 GMT

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




addons/models/0010-base_model.json
Line 105 (original), 105 (patched)
<https://reviews.apache.org/r/60938/#comment256275>

    I am not sunderstanding why some of these endpoints have the islegacyattribute on one
end and some have it on both. It seems to be somehting to do with whetehr the relationships
is composition or aggregation ; in that case the container end has the legacy flag.



addons/models/0030-hive_model.json
Line 554 (original), 554 (patched)
<https://reviews.apache.org/r/60938/#comment256278>

    why has this composition got the legacy flag on both ends ? Whereas otehr only have it
one one .



addons/models/0080-storm_model.json
Line 150 (original), 150 (patched)
<https://reviews.apache.org/r/60938/#comment256276>

    shouldn't this be aggregation ?



addons/models/0080-storm_model.json
Line 158 (original), 158 (patched)
<https://reviews.apache.org/r/60938/#comment256273>

    do we need the isLagacyAttribute here?



intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java
Line 64 (original), 63 (patched)
<https://reviews.apache.org/r/60938/#comment256267>

    is is
    
    also I think we should document what we mean by a legacy attribute and how it behaves
in different relationshionships. Also we should recommend here in the API docs when users
should or should not set this flag and why.



intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Line 99 (original), 103 (patched)
<https://reviews.apache.org/r/60938/#comment256270>

    I am not sure what this comment means. If we have the flag on both ends, we coudl generate
the label using eitehr entity; there does not seem to be any code for this case.
    
    I suggest these cases need junits



intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Line 230 (original), 259 (patched)
<https://reviews.apache.org/r/60938/#comment256271>

    same - I am not sure what this means



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
Lines 256 (patched)
<https://reviews.apache.org/r/60938/#comment256272>

    Does the API still allow creation of entities with constaints - how will this come through
to thie code.


- David Radley


On July 18, 2017, 6:25 a.m., Sarath Subramanian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60938/
> -----------------------------------------------------------
> 
> (Updated July 18, 2017, 6:25 a.m.)
> 
> 
> Review request for atlas and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-1959
>     https://issues.apache.org/jira/browse/ATLAS-1959
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> * Improve relationship model to support create/update operations and the following cardinalities
(previously supported using inverseReference):
> 1 to 1
> 1 to many
> many to 1
> many to many
> 
> 
> * Change legacyLabel flag in AtlasRelationshipEndDef to boolean flag.
> * Add unit tests for the above cases.
> 
> 
> Diffs
> -----
> 
>   addons/models/0010-base_model.json 303f3796 
>   addons/models/0030-hive_model.json a795f0f3 
>   addons/models/0050-falcon_model.json 7755fa86 
>   addons/models/0060-hbase_model.json 1d264df4 
>   addons/models/0080-storm_model.json 25360ff0 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 68da6af1 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 2de9bdf0

>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java fc820d49

>   intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java f80ea895

>   intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java e94dd190 
>   intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 841b66f7 
>   intg/src/main/java/org/apache/atlas/type/AtlasStructType.java f97d7674 
>   intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java PRE-CREATION 
>   intg/src/test/java/org/apache/atlas/TestUtilsV2.java 9774583d 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 6f6d74bc

>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java
12e8bb1f 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java
cd9a47ad 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
3ff6fbef 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
d4fdc257 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
157f8cd2 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
f4257be7 
>   repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java
de8e7ef3 
>   repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java
d9017319 
>   repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreHardDeleteV1Test.java
PRE-CREATION 
>   repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreSoftDeleteV1Test.java
PRE-CREATION 
>   repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java
67702231 
> 
> 
> Diff: https://reviews.apache.org/r/60938/diff/2/
> 
> 
> Testing
> -------
> 
> added unit test - AtlasRelationshipStoreV1Test
> 
> mvn clean package - succeeded with no errors.
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>


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