> On Aug. 20, 2017, 6:56 p.m., Madhan Neethiraj wrote:
> > intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java
> > Lines 54 (patched)
> > <https://reviews.apache.org/r/61526/diff/2/?file=1800429#file1800429line54>
> >
> >     >> we need to store the entityTypes specified in our supertypes. i.e. 
> > our parent classificationDefs may specify more entityTypes that we also 
> > need to allow
> >         
> >     This comment doesn't look correct. A classification can have restricted 
> > entity-types than its super-types.
> 
> David Radley wrote:
>     I suggest we have a choice, if we want entitytypes to be inheritted then:
>     1- subclassifications can restrict the entitytypes and not increase them. 
> This means that the super classification must always have the most 
> entitytypes or be unrestricted. In this case your proposed can.. logic works. 
>  
>     or
>     2- subclassification can increase the entitytypes and not further 
> restrict them. This means that subclassifications are always the same as 
> their parent but might be able to be applied further. For me this meets the 
> is-a criteria. the can... logic is coded with this in mind.
>     
>     or if we do not want them to be inheritted then:
>     3- we do not inherit entitytypes. What is specified is what we get.
>     
>     I think you are proposing option 1 is best. This would mean that children 
> classifcations could be less that the parent. This could be like a child 
> having less attributes than its parent; so any logic could not treat the 
> parent as having its defined attributes. 
>     
>     I think we should go with option 2; option 2 is in the spirit of UML and 
> pure inheritance; i.e. a sub type should add features; for example it can add 
> relationships or attributes. A sub type should not reduce features. If 
> ClassificationA can be applied to EntityA then all of its children 
> classifications should be able to be applied to EntityA. I am suggesting that 
> a child could be defined to apply to EntityB in addition to EntityA. 
>     
>     Option 3 is simpler but as we have inheritance - does not seem correct.
> 
> Madhan Neethiraj wrote:
>     I consider this as different from attribute/method inheritance. In this 
> case, allowing a sub-type to widen the allowed entity-types would violate the 
> restrictions specified in its super-types - especially considering that an 
> entity associated with a classification would also be implicitly associated 
> with all super-types of the classification (remember - the classification 
> includes all attributes of its super-types). Hence I think it is important to 
> have sub-types not allowed to widen the allowed entity-types.

Ok I see your thinking. So if a parent has any entityTypes - the child cannot 
specify entityTypes as [], the only valid values for the child are subsets of 
the parents entityTypes. Thanks for your quick reply.


- David


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


On Aug. 20, 2017, 11:34 a.m., David Radley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61526/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2017, 11:34 a.m.)
> 
> 
> Review request for atlas, Graham Wallis, Madhan Neethiraj, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-2029: Restrict entities, classifications can be applied to
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 
> 2503d8ef203cf4efbe15b440257b1da2252b6153 
>   
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasClassificationDef.java 
> eeaf71413a56c08db8170fd3323b8e8245ae44fe 
>   intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java 
> 56c3ed38392d2d2c8882861373cd42a549b5670d 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 
> 427439ca97d496f02de2d38329c582ded239c04c 
>   intg/src/test/java/org/apache/atlas/TestUtilsV2.java 
> fc65af057255b4c17378080ee4fb7cbfc780c3fe 
>   intg/src/test/java/org/apache/atlas/type/TestAtlasClassificationType.java 
> aaf4a6ac0a978e5eb6de41279cae1b1c82373374 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasClassificationDefStoreV1.java
>  89445048c69555b86a5347b21dde5a21dae9b2a1 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java
>  1c168b4cff0d105c7a0d4a9fbdb50871388c917e 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java
>  227f7cd12a9b23c3bbc1cfdc40d06616ea775ca4 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasTypeDefGraphStoreV1.java
>  50a421662b9a58ea3daf45e57d21626b5f2c6c44 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java
>  8638a7f17e8a17d3a6e0bfb94879b5c5406be1a3 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java
>  62fad5b02a7229d9bc3083690980eb063756bc29 
> 
> 
> Diff: https://reviews.apache.org/r/61526/diff/2/
> 
> 
> Testing
> -------
> 
> Testing
> performed unit tests
> 
> Using postman
> 1) create an entityDef  aaa
> 2) create a classificationDef with an entitytype  aaaa - checked that it is 
> in the response
> 3) Create a entity instance of aaa
> 4) add the classification to it
> 5) Create an entity instance with a different type bbbb
> 6) Attempt to add the classification to bbbb. this fails with an informative 
> message
> 7) Attempt to update the ClassificationDef to remove the entity type - this 
> fails with an informative message
> 8) Attempt to update the classificationdef to add bbbb. this update works. 
> 9) Attempt to add an entity type that does not exist to the 
> ClassificationDef. this should fail.  
> 10) Attempt to update an entity type that does not exist to the 
> ClassificationDef. this should fail.
> 
> 
> Thanks,
> 
> David Radley
> 
>

Reply via email to