----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66928/#review203462 -----------------------------------------------------------
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/AtlasGraphSONReader.java Line 315 (original), 332 (patched) <https://reviews.apache.org/r/66928/#comment285691> shouldn't this return 'this.propertiesToPostProcess', instead of creating a new instance? If this method is used only to initialize, either rename it or avoid this method by directly initializing in line #252. graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/GraphSONUtility.java Line 67 (original), 74 (patched) <https://reviews.apache.org/r/66928/#comment285696> This one handles removal/updte of properties for map type attributes. Aren't similar updates needed for array type attributes as well (primitive/struct/entity-ref)? graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/GraphSONUtility.java Lines 152 (patched) <https://reviews.apache.org/r/66928/#comment285693> edges for all array elements will have __index set to 0. Does this get updated in subsequent processing? graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/GraphSONUtility.java Lines 340 (patched) <https://reviews.apache.org/r/66928/#comment285694> Method replaceReferencedEdgeIdForList() and postProcessListProperty don't seem to be used. Please review and remove. graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/postProcess/PostProcessListProperty.java Lines 43 (patched) <https://reviews.apache.org/r/66928/#comment285703> Should newIdPositionMap be cleared here? graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/postProcess/PostProcessor.java Lines 80 (patched) <https://reviews.apache.org/r/66928/#comment285688> doesNotHaveProperty() - the name doesn't seem to match the condition "return v.property(Constants.TYPENAME_PROPERTY_KEY).isPresent()", which will return true for all entity/classification vertices. Please review. graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/postProcess/PostProcessor.java Lines 84 (patched) <https://reviews.apache.org/r/66928/#comment285689> isVertexOfType() ==> isInstanceVertexOfType() - Madhan Neethiraj On May 17, 2018, 8:48 p.m., Ashutosh Mestry wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66928/ > ----------------------------------------------------------- > > (Updated May 17, 2018, 8:48 p.m.) > > > Review request for atlas, Madhan Neethiraj, Ruchi Solani, and Sarath > Subramanian. > > > Bugs: ATLAS-2637 > https://issues.apache.org/jira/browse/ATLAS-2637 > > > Repository: atlas > > > Description > ------- > > **Approach** > New class: _TypesStoringEdgeIds_: Navigates through all the types in the > _typeRegistry_, returns map of entity type and properties that store edge ids. > Modified: _DataMigrationService_: Uses output from class above and passes it > down to migration. > Modified: _PostProcessManager_: Uses the map generated above and uses it for > post processing. > Modified: _GraphSONUtility_: Improvement to check for vertex of type. This > avoids potential exeception when a non-existent property is checked for > presence. > > Added PostProcess framework. > Added logic for handling new Array and Map representation. > > > Diffs > ----- > > > graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java > 607baf664 > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java > c0b9c1741 > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphDatabase.java > 16aecd5e2 > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/AtlasGraphSONReader.java > ae119b0bc > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/GraphSONUtility.java > ec320b03e > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/PostProcessManager.java > d0a65f7b1 > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/RelationshipTypeCache.java > e4e82649b > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/postProcess/PostProcessListProperty.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/migration/postProcess/PostProcessor.java > PRE-CREATION > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/migration/BaseUtils.java > e863d9fae > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/migration/GraphSONUtilityPostProcessTest.java > 4d73c78ef > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/migration/GraphSONUtilityTest.java > 794b5471e > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/migration/MappedElementCacheTest.java > cac09d229 > > graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/migration/PostProcessListPropertyTest.java > PRE-CREATION > graphdb/janus/src/test/resources/col-2-legacy.json PRE-CREATION > graphdb/janus/src/test/resources/edge-legacy-col.json PRE-CREATION > graphdb/janus/src/test/resources/edge-legacy-col2.json PRE-CREATION > graphdb/janus/src/test/resources/edge-legacy-col3.json PRE-CREATION > graphdb/janus/src/test/resources/edge-legacy-col4.json PRE-CREATION > graphdb/janus/src/test/resources/table-v-147504.json 898dce5df > intg/src/main/java/org/apache/atlas/store/AtlasTypeDefStore.java b05754f4b > intg/src/test/java/org/apache/atlas/TestUtilsV2.java 886ce77f5 > > repository/src/main/java/org/apache/atlas/repository/migration/DataMigrationService.java > 22cd55217 > > repository/src/main/java/org/apache/atlas/repository/migration/TypesWithCollectionsFinder.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasTypeDefGraphStoreV1.java > 3c84e3c22 > > repository/src/test/java/org/apache/atlas/repository/migration/ComplexAttributesTest.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/repository/migration/HiveParititionTest.java > ac0b79d38 > > repository/src/test/java/org/apache/atlas/repository/migration/HiveStocksTest.java > ffbf3200b > > repository/src/test/java/org/apache/atlas/repository/migration/MigrationBaseAsserts.java > ec6e64a25 > > repository/src/test/java/org/apache/atlas/repository/migration/TypesWithCollectionsFinderTest.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityTestBase.java > d810a1166 > repository/src/test/resources/complex-attr_db/atlas-migration-data.json > PRE-CREATION > repository/src/test/resources/complex-attr_db/atlas-migration-typesdef.json > PRE-CREATION > repository/src/test/resources/parts_db/atlas-migration-data.json 1414ea160 > > > Diff: https://reviews.apache.org/r/66928/diff/16/ > > > Testing > ------- > > **Unit tests** > > Additional tests added. > > **Functional tests** > > Regular flow verified. > > **[Pre-commit > build](https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/416/)** > > > Thanks, > > Ashutosh Mestry > >
