atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Madhan Neethiraj <mad...@apache.org>
Subject Re: Review Request 66928: Data Migration: Import: Infer Types that Store Edge Ids
Date Sat, 19 May 2018 00:36:35 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message