----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55531/#review161657 -----------------------------------------------------------
I am yet to complete the review; here are few comments so far. intg/src/main/java/org/apache/atlas/type/AtlasClassificationType.java (line 54) <https://reviews.apache.org/r/55531/#comment232943> Consider the following alternative: - Introduce class "org.apache.atlas.type.AtlasAttribute", which encapsulates AtlasAttributeDef: public AtlasAttribute(AtasStructDef structDef, AtlasAttributeDef attrDef) { this.structDef = structDef; this.attrDef = attrDef; this.qualifiedAttributeName = structDef.getName() + "." + attrDef.getName(); } public AtlasAttributeDef getAttributeDef() { return attrDef; } public String getQualifiedAttributeName() { return qualifiedAttributeName; } - Change type of "allAttributeDef" as below: Map<String, AtlasAttribute> allAttributeDefs = Collections.emptyMap(); This will be a good place to keep attribute specific additional functionalties (like validation, constraints, ..) - Remove "allQualifiedAttributeNames" intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java (line 176) <https://reviews.apache.org/r/55531/#comment232946> When obj is of type AtlasEntity or AtlasObjectId, consider adding validation of the "typeName" - to ensure that the instance type matches the AtlasEntityType. intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java (line 178) <https://reviews.apache.org/r/55531/#comment232944> "obj instanceof AtlasObjectId" - consider moving this to before line #172 "for (AtlasEntityType superType : superTypes) {". intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java (line 186) <https://reviews.apache.org/r/55531/#comment232945> What is the usecase to treat a string (guid?) as an entity? Shouldn't AtlasObjectId be used? intg/src/main/java/org/apache/atlas/type/AtlasStructType.java (line 462) <https://reviews.apache.org/r/55531/#comment232926> this requires that "." is not a valid character in attribute name. Please add validation during type-creation. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java (line 69) <https://reviews.apache.org/r/55531/#comment232947> rename getPropertyKey() ==> getTypeDefPropertyKey()? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java (line 97) <https://reviews.apache.org/r/55531/#comment232948> - consider using "AtlasStructType" for the first parameter and avoid "throws AtlasBaseException - would "fromType" be more appropriate name for "parentType"? Please review. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java (line 147) <https://reviews.apache.org/r/55531/#comment232949> Consider renaming "element" to "vertex" - Madhan Neethiraj On Jan. 14, 2017, 3:57 a.m., Suma Shivaprasad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55531/ > ----------------------------------------------------------- > > (Updated Jan. 14, 2017, 3:57 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 > repository/src/main/java/org/apache/atlas/RepositoryMetadataModule.java > 54dda50 > > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java > be9fed2 > > repository/src/main/java/org/apache/atlas/discovery/EntityLineageService.java > 45e2dd2 > repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java > bcdf344 > > 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/graph/GraphBackedMetadataRepositoryTest.java > 7444bf3 > > 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/AtlasInstanceRestAdapters.java > ad16be7 > webapp/src/main/java/org/apache/atlas/web/rest/EntitiesREST.java f6acd07 > > 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 > >
