----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59769/#review178356 -----------------------------------------------------------
authorization/src/main/java/org/apache/atlas/authorize/AtlasResourceTypes.java Line 22 (original), 22 (patched) <https://reviews.apache.org/r/59769/#comment252264> This should be RELATIONSHIP authorization/src/main/java/org/apache/atlas/authorize/simple/AtlasAuthorizationUtils.java Lines 142 (patched) <https://reviews.apache.org/r/59769/#comment252265> This should be relationship authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyParser.java Lines 234 (patched) <https://reviews.apache.org/r/59769/#comment252266> same graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1Graph.java Line 408 (original), 403 (patched) <https://reviews.apache.org/r/59769/#comment252270> I am wondering what the intent is here- please could you add a comment? intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java Lines 227 (patched) <https://reviews.apache.org/r/59769/#comment252271> Should relationship equals only check the structural parts. Not createdBy , updatetime etc. intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java Line 145 (original), 169 (patched) <https://reviews.apache.org/r/59769/#comment252272> In the RelationshipDef review - I was advised to use getIsContainter in case the json aparsing needs this. intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java Line 147 (original), 171 (patched) <https://reviews.apache.org/r/59769/#comment252273> setContainer -> setIsContainer intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java Lines 131 (patched) <https://reviews.apache.org/r/59769/#comment252274> the method name is isTypeOrSuperTypeOf, but the check is of subtypes. Is this what you intended? intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java Lines 60 (patched) <https://reviews.apache.org/r/59769/#comment252275> can relationshipDef be null here? intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java Lines 61 (patched) <https://reviews.apache.org/r/59769/#comment252276> if we need to check relationshipDef not null- I suggest we do it only once. intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java Lines 63 (patched) <https://reviews.apache.org/r/59769/#comment252278> if an end type is null -shouldn't we stop any firther processing in this method. intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java Lines 66 (patched) <https://reviews.apache.org/r/59769/#comment252277> Rather than use reflection I suggest checking the category using getTypeCategory. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java Lines 101 (patched) <https://reviews.apache.org/r/59769/#comment252279> So we create the first edge of the relationship and then additional legacy edges. Will any legacy implmnetations be effected by the new edge? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java Lines 106 (patched) <https://reviews.apache.org/r/59769/#comment252280> I am not sure this is right. It is valid to have more than one relationship between 2 entities. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java Lines 185 (patched) <https://reviews.apache.org/r/59769/#comment252281> unknown relationship ==> unknown relationshiptype repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java Lines 197 (patched) <https://reviews.apache.org/r/59769/#comment252282> in the Def code there is a get method that then works out if it needs to do a getbyName or get bu Guid. I suggest following the same pattern. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java Lines 222 (patched) <https://reviews.apache.org/r/59769/#comment252283> I assume we need to validate the attribute name as well as the AtlasObjectId content. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java Lines 264 (patched) <https://reviews.apache.org/r/59769/#comment252284> I suggest the default version be a constant. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java Lines 301 (patched) <https://reviews.apache.org/r/59769/#comment252285> where do we add the relationship attributes ? - David Radley On June 20, 2017, 1:55 a.m., Sarath Subramanian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59769/ > ----------------------------------------------------------- > > (Updated June 20, 2017, 1:55 a.m.) > > > Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, > Nixon Rodrigues, and Suma Shivaprasad. > > > Bugs: ATLAS-1856 > https://issues.apache.org/jira/browse/ATLAS-1856 > > > Repository: atlas > > > Description > ------- > > Based on the relationDef implementation in ATLAS-1852. Implement instance API > and REST for entity relationships. This patch is not complete. > work in progress > > > Diffs > ----- > > > authorization/src/main/java/org/apache/atlas/authorize/AtlasResourceTypes.java > deccf843 > > authorization/src/main/java/org/apache/atlas/authorize/simple/AtlasAuthorizationUtils.java > 93d988e6 > > authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyParser.java > 7ef49e66 > > graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphQuery.java > bd7b35e4 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/NativeTitanGraphQuery.java > 662a2705 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/titan/query/TitanGraphQuery.java > 056088cf > > graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/NativeTitan0GraphQuery.java > 5ad176b5 > > graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1Graph.java > e829d918 > > graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/query/NativeTitan1GraphQuery.java > 9dc175b6 > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 6c33f402 > intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java > 000d747b > intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java 0ff15827 > intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java > eb2fc48b > intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java aebd4d1a > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java > ca7fad06 > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasRelationshipStore.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java > 560b3385 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java > 66f20daa > > repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java > 7a064b60 > repository/src/test/java/org/apache/atlas/TestModules.java 095af417 > webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/59769/diff/6/ > > > Testing > ------- > > Tested using POSTMAN. UTs and ITs in progress. > > > Thanks, > > Sarath Subramanian > >
