----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55531/#review162252 -----------------------------------------------------------
intg/src/main/java/org/apache/atlas/type/AtlasStructType.java (line 264) <https://reviews.apache.org/r/55531/#comment233582> I think 'string' should not be a valid value for a Struct/Entity/Classification. If there is no usecase to allow this value, please remove this. intg/src/main/java/org/apache/atlas/type/AtlasStructType.java (line 266) <https://reviews.apache.org/r/55531/#comment233583> AtlasObjectId should be allowed only when attribute-type is a entity. It wouldn't be a valid value for struct or classficaition. Please review. repository/src/main/java/org/apache/atlas/repository/store/graph/EntityGraphDiscoveryContext.java (line 111) <https://reviews.apache.org/r/55531/#comment233589> unresolvedEntityReferences is type List<AtlasEntity>. would calling remove(AtlasObjectId id) result in correct removal? Please review. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/StructVertexMapper.java (line 56) <https://reviews.apache.org/r/55531/#comment233602> Consider marking graph, mapVertexMapper and arrVertexMapper as final. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/StructVertexMapper.java (line 157) <https://reviews.apache.org/r/55531/#comment233603> Should vertex be cleaned of properties that are not present in value? updateTime, updatedBy system attributes? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/StructVertexMapper.java (line 174) <https://reviews.apache.org/r/55531/#comment233604> Shouldn't createdBy and updatedBy attributes be set here? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/UniqAttrBasedEntityResolver.java (line 46) <https://reviews.apache.org/r/55531/#comment233596> Consider marking 'typeRegistry' and 'graphHelper' as final. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/UniqAttrBasedEntityResolver.java (line 55) <https://reviews.apache.org/r/55531/#comment233598> Consider moving this to line #49, along with other attributes. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/UniqAttrBasedEntityResolver.java (line 93) <https://reviews.apache.org/r/55531/#comment233599> after the validation in line #83, there shouldn't be any unresolvedEntityReference here, right? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/UniqAttrBasedEntityResolver.java (line 107) <https://reviews.apache.org/r/55531/#comment233600> String qualifiedAttrName = attr.getQualifiedName(); repository/src/main/java/org/apache/atlas/repository/store/graph/v1/UniqAttrBasedEntityResolver.java (line 114) <https://reviews.apache.org/r/55531/#comment233601> Please add if (LOG.isDebugEnabled()) server-api/src/main/java/org/apache/atlas/RequestContextV1.java (line 47) <https://reviews.apache.org/r/55531/#comment233591> Consider setting this to final and initialize in the constructor. server-api/src/main/java/org/apache/atlas/RequestContextV1.java (line 59) <https://reviews.apache.org/r/55531/#comment233590> since CURRENT_CONTEXT is a ThreadLocal<>, synchronized() usage is not necessary. Please review. A simpler impl could be: public static RequestContextV1 get() { RequestContextV1 ret = CURRENT_CONTEXT.get(); if (ret == null) { ret = new RequestContextV1(); CURRENT_CONTEXT.set(ret); } return ret; } server-api/src/main/java/org/apache/atlas/RequestContextV1.java (line 106) <https://reviews.apache.org/r/55531/#comment233594> Any reason to create a new list here, instead of simply returning 'createdEntityIds'? The return can be changed to Collection<String>. Please review other methods here for this suggestion. - Madhan Neethiraj On Jan. 19, 2017, 6:22 a.m., Suma Shivaprasad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55531/ > ----------------------------------------------------------- > > (Updated Jan. 19, 2017, 6:22 a.m.) > > > Review request for atlas. > > > Bugs: ATLAS-1467 > https://issues.apache.org/jira/browse/ATLAS-1467 > > > Repository: atlas > > > Description > ------- > > Implemented > > 1. Entity graph discovery - This differs from current implementation in that > it collects all references first and resolves them later . The current > implemenation tries to resolve references at the same time resulting in > issues when entities have cross references to each other . Also it handles > resolving a refernce by its unique attribute (by supplying an AtlasEntity > instead of an Id) > 2. Create with new APIs and POJOs ( no change in graph model ) > 3. Full Update with new APIs and POJOs ( no change in graph model ) > > > Diffs > ----- > > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 6770c41 > intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 2ad0f76 > intg/src/main/java/org/apache/atlas/model/instance/AtlasEntityHeader.java > e7b70aa > intg/src/main/java/org/apache/atlas/model/instance/EntityMutations.java > 3501c90 > intg/src/main/java/org/apache/atlas/model/typedef/AtlasEntityDef.java > 85e9d77 > intg/src/main/java/org/apache/atlas/model/typedef/AtlasStructDef.java > aee26ef > intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java > 8772720 > intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java 3625f72 > intg/src/main/java/org/apache/atlas/type/AtlasStructType.java e20af76 > intg/src/test/java/org/apache/atlas/TestUtilsV2.java 53b109c > intg/src/test/java/org/apache/atlas/type/TestAtlasTypeRegistry.java 9429c07 > repository/src/main/java/org/apache/atlas/RepositoryMetadataModule.java > 54dda50 > > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java > 2be9a2d > > repository/src/main/java/org/apache/atlas/discovery/EntityLineageService.java > 45e2dd2 > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java > 5259249 > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java > f17b816 > > repository/src/main/java/org/apache/atlas/repository/store/graph/EntityGraphDiscovery.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/EntityGraphDiscoveryContext.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/EntityResolver.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/ArrayVertexMapper.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java > e731c11 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEnumDefStoreV1.java > fccbeba > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java > 18b3b85 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java > e780dc1 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityMutationContext.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/GraphMutationContext.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/IDBasedEntityResolver.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/InstanceGraphMapper.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/MapVertexMapper.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/SoftDeleteHandlerV1.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/StructVertexMapper.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/UniqAttrBasedEntityResolver.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/util/AtlasRepositoryConfiguration.java > ea0e670 > > repository/src/test/java/org/apache/atlas/lineage/EntityLineageServiceTest.java > b1dac9d > > repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStoreTest.java > c7c3286 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java > PRE-CREATION > server-api/src/main/java/org/apache/atlas/RequestContextV1.java > PRE-CREATION > > webapp/src/main/java/org/apache/atlas/web/adapters/AtlasEntityFormatConverter.java > c4be236 > > webapp/src/main/java/org/apache/atlas/web/adapters/AtlasInstanceRestAdapters.java > ad16be7 > > webapp/src/main/java/org/apache/atlas/web/adapters/AtlasStructFormatConverter.java > 3565ab3 > webapp/src/main/java/org/apache/atlas/web/rest/EntitiesREST.java f6acd07 > webapp/src/main/java/org/apache/atlas/web/util/LineageUtils.java 54ca236 > webapp/src/test/java/org/apache/atlas/web/adapters/TestEntitiesREST.java > 265b650 > webapp/src/test/java/org/apache/atlas/web/adapters/TestEntityREST.java > 2a75773 > webapp/src/test/java/org/apache/atlas/web/resources/BaseResourceIT.java > dcb1264 > > webapp/src/test/java/org/apache/atlas/web/resources/EntityDiscoveryJerseyResourceIT.java > 2ade5b0 > > webapp/src/test/java/org/apache/atlas/web/resources/EntityV2JerseyResourceIT.java > d7702e2 > > Diff: https://reviews.apache.org/r/55531/diff/ > > > Testing > ------- > > UTs for create > > TODO: > Update testing pending > Error handling for unresolved references > UTS for discovery > > > Thanks, > > Suma Shivaprasad > >
