----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59719/#review177505 -----------------------------------------------------------
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java Lines 76 (patched) <https://reviews.apache.org/r/59719/#comment251099> Consider renaming "RELATIONSHIPDEF_NON_PRIMITIVE_ATTRIBUTE" to "RELATIONSHIPDEF_UNSUPPORTED_ATTRIBUTE_TYPE" intg/src/main/java/org/apache/atlas/AtlasErrorCode.java Lines 121 (patched) <https://reviews.apache.org/r/59719/#comment251100> INVALID_RELATIONSHIP_ISCONTAINER_TWICE seems to duplicate RELATIONSHIPDEF_DOUBLE_CONTAINERS. Please review. intg/src/main/java/org/apache/atlas/model/TypeCategory.java Line 21 (original), 21 (patched) <https://reviews.apache.org/r/59719/#comment251101> It is a good practice to add enum lements at the end of existing ones. intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java Lines 68 (patched) <https://reviews.apache.org/r/59719/#comment251107> AtlasRelationshipType.resolveReferences() should: - validate that endPointDef1.type and endPointDef2.type refer to known entity-types - validate that endPointDef1.name and endPointDef2.name don't conflict with existing attributes/relationship-attributes in the entity-types - perform validations currently being done in AtlasRelationshipDef repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasRelationshipType.java Lines 45 (patched) <https://reviews.apache.org/r/59719/#comment251111> AtlasRelationshipType is already defined in intg module. This version in repository module should be removed. repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java Line 447 (original), 493 (patched) <https://reviews.apache.org/r/59719/#comment251112> preDeleteRelationshipDefs is not populated before referencing in line #501, #503. Add a block to populate preDeleteRelationshipDefs - similar to the one for preDeleteEntityDefs in lines #484 - #492. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 46 (patched) <https://reviews.apache.org/r/59719/#comment251117> Is 'modelCategory' member necessary? The value will always be TypeCategory.RELATIONSHIP for all instance of this class. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 100 (patched) <https://reviews.apache.org/r/59719/#comment251113> Create edges to types referenced by attributes (like enumDefs) with the following call: - AtlasStructDefStoreV1.updateVertexAddReferences(structDef, vertex, typeDefStore); In addition, create edges to types referenced by endPointDef1.type and endPointDef1.type. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 117 (patched) <https://reviews.apache.org/r/59719/#comment251114> TypeCategory.CLASS ==> TypeCategory.RELATIONSHIP repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 136 (patched) <https://reviews.apache.org/r/59719/#comment251115> TypeCategory.CLASS ==> TypeCategory.RELATIONSHIP repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 159 (patched) <https://reviews.apache.org/r/59719/#comment251116> TypeCategory.CLASS ==> TypeCategory.RELATIONSHIP repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 208 (patched) <https://reviews.apache.org/r/59719/#comment251122> TypeCategory.CLASS ==> TypeCategory.RELATIONSHIP repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 215 (patched) <https://reviews.apache.org/r/59719/#comment251119> update references to other types: - for addition of new attributes - changes in endPointDef1.type, endPointDef2.type repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 240 (patched) <https://reviews.apache.org/r/59719/#comment251123> TypeCategory.CLASS ==> TypeCategory.RELATIONSHIP repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 247 (patched) <https://reviews.apache.org/r/59719/#comment251120> update references to other types: - for addition of new attributes - changes in endPointDef1.type, endPointDef2.type repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 263 (patched) <https://reviews.apache.org/r/59719/#comment251124> Isn't enough to look for TypeCategory.RELATIONSHIP - why look for TypeCategory.CLASS, then fall back to look for TypeCategory.RELATIONSHIP. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 268 (patched) <https://reviews.apache.org/r/59719/#comment251121> >> AtlasGraphUtilsV1.typeHasInstanceVertex() For relationships, we should look for presence fo any edges, not vertex, with typeName == name. Please review. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 312 (patched) <https://reviews.apache.org/r/59719/#comment251125> Isn't enough to look for TypeCategory.RELATIONSHIP - why look for TypeCategory.CLASS, then fall back to look for TypeCategory.RELATIONSHIP. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 319 (patched) <https://reviews.apache.org/r/59719/#comment251126> >> AtlasGraphUtilsV1.typeHasInstanceVertex() For relationships, we should look for presence fo any edges, not vertex, with typeName == name. Please review. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java Lines 361 (patched) <https://reviews.apache.org/r/59719/#comment251127> Haven't seen use of setJsonProperty()/getJsonProperty() before. I think we might have better control with one of the following approaches: - save Json-ified string (of endPointDef1, endPointDef2) in string typed vertex property - save fields of endPointDef1, endPointDef2 in individual vertex properties, like: endPointDef1.type, endPointDef1.name, endPointDef1.isContainer, endPointDef1.cardinality, endPointDef2.type, .. - Madhan Neethiraj On June 9, 2017, 1:22 p.m., David Radley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59719/ > ----------------------------------------------------------- > > (Updated June 9, 2017, 1:22 p.m.) > > > Review request for atlas, Graham Wallis, Madhan Neethiraj, and Sarath > Subramanian. > > > Repository: atlas > > > Description > ------- > > This patch introduces the relationshipDef as a new top level TypeDef, that is > stored as a vertex in the graph. Other subtasks are required to complete the > Relationshipdef work. > > 1) relationshipdef updates and deletes have not been sueccessfully tested > 2) further constraints are required - around checking exising EntityDefs and > RelationshipDefs for consistancy. This piece will not be handled in this > subtask > 3) Creation of edges between xxxDef vertexes. I will update the design with a > proposal > > > Diffs > ----- > > common/src/main/java/org/apache/atlas/repository/Constants.java > bcdf08cdfbf1d4d8689d3d79413b2ff181b621a4 > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java > d723b2a9fe03245f78bf9af53058aaa801e62aff > intg/src/main/java/org/apache/atlas/model/TypeCategory.java > e47a8a7dab0aac6154833a58148412590be6f796 > intg/src/main/java/org/apache/atlas/model/typedef/AtlasBaseTypeDef.java > 7308eb73b513660affaf35b944556d7076289815 > intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndPointDef.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/model/typedef/AtlasTypesDef.java > af95bff5b53bf14057c53820cc62255d37c50498 > intg/src/main/java/org/apache/atlas/store/AtlasTypeDefStore.java > 198bd8fe515a96e654b24de3af92b6edfac3a6ae > intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/type/AtlasTypeRegistry.java > 1b3526bfcc7d13aa397844c5dec55e34dbc8ed7e > intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java > c0135f524b2ee926fb94aae31e6b49dab424a19a > intg/src/test/java/org/apache/atlas/model/ModelTestUtil.java > 084bcc4609591fd24dc0ee79290be1b337068e6a > > intg/src/test/java/org/apache/atlas/model/typedef/TestAtlasRelationshipDef.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasRelationshipDefStore.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasRelationshipType.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java > 17b7e17742de97bb9de2a4b375fea3c58b75e26b > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipDefStoreV1.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasTypeDefGraphStoreV1.java > f0c83806980153bab8a31647281015376a9d2168 > > repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java > 7a064b600cb6b3e02a814f45370fcc9041ebcd7e > repository/src/test/scala/org/apache/atlas/query/QueryTestsUtils.scala > c844558a9463d0953274ba28c54e08272a93ce89 > typesystem/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java > 21d5f1a1e7488c73ab84ec9512d488ed3b9002bf > > typesystem/src/main/java/org/apache/atlas/typesystem/types/RelationshipEndPointDef.java > PRE-CREATION > > typesystem/src/main/java/org/apache/atlas/typesystem/types/RelationshipTypeDefinition.java > PRE-CREATION > typesystem/src/main/java/org/apache/atlas/typesystem/types/TypeSystem.java > 262f784f50805c8fccde77cfe739e8538b49ab8d > > typesystem/src/main/java/org/apache/atlas/typesystem/types/utils/TypesUtil.java > f131458fc36dd4e9c29b49ce903446526d020877 > typesystem/src/main/scala/org/apache/atlas/typesystem/TypesDef.scala > b51048df2c9bccf904ffd5287d5021d2294fe458 > > typesystem/src/main/scala/org/apache/atlas/typesystem/builders/TypesBuilder.scala > 5ea345feeee79dec15c9fa5cd27724aedf50eaae > > typesystem/src/main/scala/org/apache/atlas/typesystem/json/TypesSerialization.scala > 4478a44b55f745076c9f15a47371b3863ca56c9c > webapp/src/main/java/org/apache/atlas/web/rest/TypesREST.java > c32f36ea3a5025d2cec11b6ac0bdfe192e9c05f9 > > > Diff: https://reviews.apache.org/r/59719/diff/2/ > > > Testing > ------- > > Junits show an error. I am publishing this patch - now at master, for more > feedback. I will investigate. > 1) create relationshipDef > 2) get typedefs > 3) get typedef headers > 4) get relationshgipdef by name > 5) get relationshipDef by guid. > > There is code in for delete - but it does not appear to be effective. I am > continuing to invesigate > There is code in for update - but this is failing as the delete is not > working I am continuing to invesigate > > > Thanks, > > David Radley > >
