----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63879/#review193913 -----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java Line 205 (original), 208 (patched) <https://reviews.apache.org/r/63879/#comment272573> The called method itself already traces this. The => is misleading as it is not an entry. I suggest removing this trace entry repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java Lines 289 (patched) <https://reviews.apache.org/r/63879/#comment272574> I suggest you trace args - as they could be very good for debugging repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java Lines 524 (patched) <https://reviews.apache.org/r/63879/#comment272575> How aboiut putting put the clazz name here? repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java Line 521 (original), 570 (patched) <https://reviews.apache.org/r/63879/#comment272576> A minor suggestion : maybe "from edge {}" repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java Line 570 (original), 630 (patched) <https://reviews.apache.org/r/63879/#comment272577> edge => edgeString repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java Lines 641 (patched) <https://reviews.apache.org/r/63879/#comment272578> should be use the vertexString method here? repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java Line 591 (original), 653 (patched) <https://reviews.apache.org/r/63879/#comment272579> and here repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java Line 764 (original), 833 (patched) <https://reviews.apache.org/r/63879/#comment272580> can we put the classType out here? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java Line 364 (original), 361 (patched) <https://reviews.apache.org/r/63879/#comment272584> I suggest using block comment rather than lots of line comments. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java Lines 365 (patched) <https://reviews.apache.org/r/63879/#comment272581> I hada go at reqeriting to remove the yous from the description. what do you think of: "This code needs to cater for imports, where entities can contain attributes that refer to entities that have not been processed yet by the import. In this situation a partial entity comes into this method; the partial entity is not pully formed becase it has a reference to an entity that the import has not seen yet. In this case getRelationshipAttributes() does not get the relationshipattributes; the code needs to use the more general getAttribute() instead. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AttributeMutationContext.java Lines 91 (patched) <https://reviews.apache.org/r/63879/#comment272582> shouldnt we use vertexString() here? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AttributeMutationContext.java Lines 95 (patched) <https://reviews.apache.org/r/63879/#comment272583> shouldnt we use edgeString() here? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java Lines 866 (patched) <https://reviews.apache.org/r/63879/#comment272585> should we mention the AtlasRelatedObjectId processing here? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java Lines 1029 (patched) <https://reviews.apache.org/r/63879/#comment272586> If we have a relationship guid - we trace it. I was expecting it to be involved with the matching of new and existing edges. - David Radley On Dec. 8, 2017, 12:53 p.m., Graham Wallis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63879/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2017, 12:53 p.m.) > > > Review request for atlas. > > > Repository: atlas > > > Description > ------- > > ATLAS-2262.patch > This patch contains fixes for the EntityGraphMapper and related classes, to > fix intermittent relationship failures. > This change means that an UPDATE operation will inspect the existing and new > relationships and compare them fully before adding/modifying edges. Matching > elements will be reused, new elements will be added, and redundant elements > will be discarded. The behaviour should be consistent regardless of which > graph provider is being used. > > > Diffs > ----- > > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java > 3e602431400677cbe0d8fe440732b02bb4a30a62 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java > d6effbf77b7dbd3479bab3899730bdcbabdce351 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java > b0f9845d6e92a6ef4bb53cccec537a0a54afb5e8 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AttributeMutationContext.java > b6d82dd834b8cc8b3bac630c084b91ac34dd0cec > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java > f6a15b695e676bc035ce8336bcce7bacf1852ff9 > > repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java > ab25faaa9caffe5bf2796194057f0556d4bfc431 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java > 1f8b9fdbfd5d987288983f7be99ea0f36f7255d4 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java > b418fef90a0bcadba47679028c134b836bdd9079 > > > Diff: https://reviews.apache.org/r/63879/diff/4/ > > > Testing > ------- > > Build with test enabled, using Janus graph provider > > > Thanks, > > Graham Wallis > >
