----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71108/#review217150 -----------------------------------------------------------
intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java Lines 688 (patched) <https://reviews.apache.org/r/71108/#comment304417> is this comment needed here? it is repeated in #691 below. intg/src/main/java/org/apache/atlas/type/AttributeToken.java Line 34 (original), 42 (patched) <https://reviews.apache.org/r/71108/#comment304416> Consider reorganizing this method to have a single 'return', like: public String eval(AtlasEntity entity, EntityFinder entityFinder) { String ret = null; try { Object attrVal = entity.getAttribute(attrName); if (attrVal == null) { AtlasEntity fullEntity = entityFinder.getFullEntity(entity); if (fullEntity != null) { attrVal = fullEntity.getAttribute(attrName); } } if (attrVal != null) { ret = attrVal.toString(); } } catch (Exception excp) { LOG.warn("AttributeToken.eval(attrName={}) failed", attrName, excp); } return ret; } intg/src/main/java/org/apache/atlas/type/DependentToken.java Line 44 (original), 47 (patched) <https://reviews.apache.org/r/71108/#comment304418> Consider reorganizing this method to have a single 'return', like: public String eval(AtlasEntity entity, EntityFinder entityFinder) { String ret = null; try { AtlasEntity curr = entity; for (String relationshipKey : objectPath) { Object attrVal = getRelationshipValue(curr, relationshipKey); if (attrVal == null) { attrVal = entityFinder.getRelatedObjectId(curr, relationshipKey); } AtlasObjectId objId = toAtlasObjectId(attrVal); if (objId == null) { break; } curr = entityFinder.getEntity(objId); if (curr == null) { break; } } Object attrVal = curr != null ? curr.getAttribute(attrName) : null; if (attrVal != null) { ret = attrVal.toString() } } catch (Exception excp) { LOG.warn("DependentToken.eval(path={}) failed", path, excp); } return ret; } intg/src/main/java/org/apache/atlas/type/DependentToken.java Lines 56 (patched) <https://reviews.apache.org/r/71108/#comment304419> entity => curr intg/src/main/java/org/apache/atlas/utils/EntityFinder.java Lines 25 (patched) <https://reviews.apache.org/r/71108/#comment304420> References to entity-stream, resolved-references in method names don't seem necessary at the interface level. Please review to simply this, like: public interface EntityFinder { AtlasEntity getEntity(String guid); AtlasEntity getEntity(AtlasObjectId objId); AtlasEntity getEntity(AtlasEntity entity); AtlasObjectId getRelatedObjectId(String entityGuid, String relationshipKey); } repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityFinder.java Lines 92 (patched) <https://reviews.apache.org/r/71108/#comment304421> Please update to handle the following cases as well: - if entity.guid is internal, this method should look into discoveryContext.getResolvedGuids() to find the vertex - if entity.guid is null, this method should use discoveryContext.getResolvedIdsByUniqAttribs() to find the vertex - Madhan Neethiraj On Aug. 8, 2019, 11:54 p.m., Merryle Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71108/ > ----------------------------------------------------------- > > (Updated Aug. 8, 2019, 11:54 p.m.) > > > Review request for atlas, Ashutosh Mestry, Aadarsh Jajodia, Sridhar K, Le Ma, > Madhan Neethiraj, and Sarath Subramanian. > > > Repository: atlas > > > Description > ------- > > Dynamic Attributes for the current entity are generated and set. > > > Diffs > ----- > > intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java > 884447f81c57faf917a8d0565fc0a0c7ebbd99f0 > intg/src/main/java/org/apache/atlas/type/AttributeToken.java > 658cf865ec1fe4eb326930024c4f189f59df0e7e > intg/src/main/java/org/apache/atlas/type/ConstantToken.java > 5ba54ae6333440b633e3b6a1175b384676004cf9 > intg/src/main/java/org/apache/atlas/type/DependentToken.java > c1c7d3df70118dd127b74d4ece65a47185258bc4 > intg/src/main/java/org/apache/atlas/type/TemplateToken.java > fffbc2ae3ff72144a2779437c600f42d684cf548 > intg/src/main/java/org/apache/atlas/utils/EntityFinder.java PRE-CREATION > intg/src/test/java/org/apache/atlas/type/TestAtlasEntityType.java > 742970390b3d64b498ff972915849ab309594c9a > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java > 01b8d196b354cf089fe4b040e396761c9a19a7eb > > repository/src/main/java/org/apache/atlas/repository/store/graph/EntityGraphDiscoveryContext.java > 2221ac4f421fbdbb2d8d39889863223568a3ae70 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityFinder.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityGraphDiscoveryV2.java > 84bb2a363358d213611d321c01cb5f14078ae09d > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java > 983d4fa7250aec40f0b83d586a5a71a063bbed62 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStream.java > c823b20079ab9ff61fc0ff9b59ccb6b301c0664d > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStreamForImport.java > 6bf962e8b206b0bc320f3268e269e9292de08b84 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityImportStream.java > cf7ac28f9d6f0d86a2c92a984a331e818a4f5f9b > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityStream.java > 41b834204b077e7a866198fead736298524787fa > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/IDBasedEntityResolver.java > fe76b3a8943180f8a4332b6363ddc782da321aa4 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/InMemoryMapEntityStream.java > e1ade0709779bfd9acc5532434bdab12c6236591 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2Test.java > a4edaf086ebc162cf6343e9ea439185f495691e6 > webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java > e5b9563d6005372e8acd11633542af59bca1ef78 > > webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerKafkaTest.java > 93675e85deb94c7b8aa4a4ca37f8abf70678ea9b > > webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java > ece46a42b77d5fb5db2b9c1b042cc5327747c1e3 > > > Diff: https://reviews.apache.org/r/71108/diff/4/ > > > Testing > ------- > > Tested with creates and updates, dynamicAttributes are calculated correctly. > Ran mvn install for both the intg and repository packages. > > > Thanks, > > Merryle Wang > >
