git.net

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Review Request 66949: ATLAS-2523: changes to accept external GUIDs and manage homeIds


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review202433
-----------------------------------------------------------




common/src/main/java/org/apache/atlas/repository/Constants.java
Lines 80 (patched)
<https://reviews.apache.org/r/66949/#comment284251>

    I suggest a comment here to describe how an Atlas developer should understand this field. 
    
    A concern I have, is that if the users go through the om aPI then the entities with a non-null homId will never be updated - apart from as part of the asynchronous cnotification process from anotehr server. The Atlas developer using the local Atlas API, needs to be aware not to change the entities and relationships with a none null homeId. Ideally we should police this.



intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java
Lines 61 (patched)
<https://reviews.apache.org/r/66949/#comment284248>

    I am wondering why you are adding a Logger and not using it om this change.



intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java
Line 384 (original), 389 (patched)
<https://reviews.apache.org/r/66949/#comment284249>

    I suggest adding a comment here, around why you are checking for '-' as the first character. 
    
    What happens if another repository creates an identifier starting with a '-' character?
    
    to cope with theis case - I am suggest that the logic should check for a null homeId and a guid starting character of '-' (or null) we should say the guid is not assigned.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Line 709 (original), 709 (patched)
<https://reviews.apache.org/r/66949/#comment284250>

    why are we leaving in commented out code


- David Radley


On May 4, 2018, 10:55 a.m., Graham Wallis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
> 
> (Updated May 4, 2018, 10:55 a.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/atlas/repository/Constants.java c570be2ccbc41a426b0f093b4a33163092223f2f 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java e6a7f19633a1642f6b415db99e20c0641df9463b 
>   intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d283fdba90b917f38eba6c937021352df 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 1b6af94f24eb7d99953f72815167c868a9bd9efd 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java edf10da9533803234342dc3313ea1024c5e7030f 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6ddaa1b74b2c81b980bcb455b614f4a4ed7 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java d51adad80e133d636ef84883d395126ae0150af5 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 3b9a287f6986ff6bca26fc20b2f94ec1700dbe1e 
> 
> 
> Diff: https://reviews.apache.org/r/66949/diff/1/
> 
> 
> Testing
> -------
> 
> Functional testing of these changes to save reference copies of entities and relationships
> 
> 
> Thanks,
> 
> Graham Wallis
> 
>