> On July 19, 2017, 3:11 p.m., David Radley wrote:
> > addons/models/0080-storm_model.json
> > Line 150 (original), 150 (patched)
> > <https://reviews.apache.org/r/60938/diff/2/?file=1778156#file1778156line150>
> >
> >     shouldn't this be aggregation ?
> 
> Sarath Subramanian wrote:
>     since a storm node can be shared acoss multiple storm topologies and not 
> tied to a single topology made it as ASSOCIATION

It seems better to have this as aggregation to should some element of 
containment. From your description , the topology contains the storm node. The 
storm node could also be contained in otehr topologies . If htis is the case - 
it should be AGGREGATION I think.


> On July 19, 2017, 3:11 p.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
> > Lines 256 (patched)
> > <https://reviews.apache.org/r/60938/diff/2/?file=1778170#file1778170line257>
> >
> >     Does the API still allow creation of entities with constaints - how 
> > will this come through to thie code.
> 
> Sarath Subramanian wrote:
>     yes, to support backward compatibility entity creation with constraints 
> is still allowed.

Thanks Sarath for the explanation. I think I get it. I wonder if there is a 
cleaner way we can design this.   

What I do not like about this design is that the user could specify 
RelationshipDefs that do not quite match the existing constraint definitions. 
Also I am not keen on an flag with legacy in - as todays new functions is 
tomorrow legacy. the models also look messy with all the duplication.       

I assume for the 'legacy case', the requirement is to be able to specify 
propagate tags- I assume there is no compelling requirement to add 
relationshipDef attributes to the old API form.

If this is the case, I suggest we remove the legacy flag from RelationshipDef 
and remove the duplicate RelationshipDefs from the models. 
 Instead we should add a flag to the constraint API form as an indicator to 
propagate tags. This would be so much cleaner - and would not need to expose 
anything legacy in the RelationshipDef API and would mean that the user could 
not mismatch relationshipDefs with existing entityDef attributeDef constraint 
definitions. The model files would also not have all the duplication that they 
currently have.


- David


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


On July 19, 2017, 7:27 p.m., Sarath Subramanian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60938/
> -----------------------------------------------------------
> 
> (Updated July 19, 2017, 7:27 p.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
>  404225c2 
>   
> 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/4/
> 
> 
> Testing
> -------
> 
> added unit test - AtlasRelationshipStoreV1Test
> 
> mvn clean package - succeeded with no errors.
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>

Reply via email to