Mailing-List: contact dev-help@atlas.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@atlas.apache.org
Content-Type: multipart/alternative;
 boundary="===============3994284099703977006=="
MIME-Version: 1.0
Subject: Re: Review Request 60938: [ATLAS-1959]: Enhance relationship
 attributes to support different cardinality mappings
From: David Radley <david_radley@uk.ibm.com>
To: Madhan Neethiraj <madhan@apache.org>
Cc: David Radley <david_radley@uk.ibm.com>,
 atlas <dev@atlas.incubator.apache.org>, Sarath Subramanian <sarath@apache.org>
Date: Wed, 19 Jul 2017 15:11:43 -0000
Message-ID: <20170719151143.20503.94654@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: David Radley <noreply@reviews.apache.org>
References: <20170718062548.11148.24597@reviews-vm2.apache.org>
In-Reply-To: <20170718062548.11148.24597@reviews-vm2.apache.org>
Reply-To: David Radley <david_radley@uk.ibm.com>
archived-at: Wed, 19 Jul 2017 15:11:52 -0000

--===============3994284099703977006==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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


--===============3994284099703977006==--