> On June 9, 2017, 4:06 p.m., Madhan Neethiraj wrote: > > intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java > > Lines 182 (patched) > > <https://reviews.apache.org/r/59719/diff/2/?file=1746756#file1746756line182> > > > > I think it will help to keep classes in model package (i.e. the ones > > used in REST inteface) simple, with no validation, etc. There is not enough > > context here to perform validatation here - like whether the types > > referenced by endPoints are valid or not. And, given these validations can > > fail during deserialization of REST call parameters, the call might not > > make to Atlas application code - hence the error message will not be in > > logs. > > > > The validations should move to > > AtlasRelationshipType.resolveReferences(), where other validations like > > references to other types can be validated.
makes sense - I have moved the validations > On June 9, 2017, 4:06 p.m., Madhan Neethiraj wrote: > > repository/src/main/java/org/apache/atlas/repository/typestore/GraphBackedTypeStore.java > > Lines 273 (patched) > > <https://reviews.apache.org/r/59719/diff/2/?file=1746770#file1746770line279> > > > > 'typesystem' is for legacy support and I think there shouldn't be a > > need to keep it updated for core model changes like introduction of > > relationships. It will save a lot of time if we keep these features > > accessible only via new V2 API. Consider reverting these changes. > > > > typesystem is currently used by DSL and notification libraries. These > > need to be reviewed and updated to use V2 equivalents (AtlasTypeRegistry). removed all of these files - apart for the typeCategory RELATIONSHIP enum. > On June 9, 2017, 4:06 p.m., Madhan Neethiraj wrote: > > webapp/src/main/java/org/apache/atlas/web/rest/TypesREST.java > > Line 22 (original), 22 (patched) > > <https://reviews.apache.org/r/59719/diff/2/?file=1746780#file1746780line22> > > > > Avoiding changes like 'collape of imports', 'whitespace only changes' > > will make reviewing a little easier and importantly will not distract from > > focusing on important changes. reverting this line gives import errors. So I think is needs more than the original imports. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59719/#review177479 ----------------------------------------------------------- On June 10, 2017, 10:22 p.m., David Radley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59719/ > ----------------------------------------------------------- > > (Updated June 10, 2017, 10: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. > This functions works > 1) create relationshipDef > 2) get typedefs > 3) get typedef headers > 4) get relationshgipdef by name > 5) get relationshipDef by guid. > 6) delete relationshipDef > > This is yet to do: > 1) create after a delete > 2) updates do not work > 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 > intg/src/test/java/org/apache/atlas/type/TestAtlasRelationshipType.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/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 > typesystem/src/main/java/org/apache/atlas/typesystem/types/DataTypes.java > 21d5f1a1e7488c73ab84ec9512d488ed3b9002bf > webapp/src/main/java/org/apache/atlas/web/resources/TypesResource.java > 08121d8d9c0ed34f62a9e4d49c4be87a98639907 > webapp/src/main/java/org/apache/atlas/web/rest/TypesREST.java > c32f36ea3a5025d2cec11b6ac0bdfe192e9c05f9 > > > Diff: https://reviews.apache.org/r/59719/diff/3/ > > > Testing > ------- > > Junits complete successfully > 1) create relationshipDef > 2) get typedefs > 3) get typedef headers > 4) get relationshgipdef by name > 5) get relationshipDef by guid. > 6) delete relationshipDef > > Delete is successful in as far as the get typedefs does not show the > relationshipDef. But a subsequent create fails as it thinks the vertex > exists. Investigating. > > > Thanks, > > David Radley > >
