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




intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Line 1192 (original), 1192 (patched)
<https://reviews.apache.org/r/75150/#comment315152>

    To improve readability, consider replacing #1192 with the following:
    
      if (value == null) {
        if (entityObj.getIsIncomplete()) {
          continue;
        }



intg/src/main/java/org/apache/atlas/type/AtlasStructType.java
Line 423 (original), 423 (patched)
<https://reviews.apache.org/r/75150/#comment315153>

    else block at #427 also needs to be updated to handle shell entity, right? 
Please review.
    
    I suggest the following:
    
    #416:  if (value != null) {
    #         ...
    #418:  } else if (!attributeDef.getIsOptional()) {
             if (entityObj.isIncomplete()) {
               continue;
             }



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java
Lines 1254 (patched)
<https://reviews.apache.org/r/75150/#comment315154>

    Looking at #1252, this block should be executed only for full-updates i.e. 
not partial update. In such case, shouldn't the vertex be updated to clear 
is-incomplete flag?
    
    Perhaps #1252 should be replaced with:
      if (!isPartialUpdate && (entity.getIsIncomplete() == null || 
!entity.getIsIncomplete())) {



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java
Line 1272 (original), 1271 (patched)
<https://reviews.apache.org/r/75150/#comment315155>

    Why is it necessary to skip adding incomplete entities to 
context.addUpdated() and context.addCreated() - #1272 and #1294 below?



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
Line 289 (original), 296 (patched)
<https://reviews.apache.org/r/75150/#comment315151>

    This change will result in change in current behavior:
    - when entity.getLabels() is null, retail all existing labels in the entity 
vertex
    - else, update entity vertext with the list of entity.getLabels(); if 
entity.getLabels() is empty, all existing labels in the vertex will be cleared
    
    I suggest to revert the change in #296; if this was done to specifically 
address an issue, please add details in the JIRA.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/IDBasedEntityResolver.java
Lines 41 (patched)
<https://reviews.apache.org/r/75150/#comment315149>

    entityGraphMapper can be marked final.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/IDBasedEntityResolver.java
Lines 78 (patched)
<https://reviews.apache.org/r/75150/#comment315148>

    "RequestContext.get().isImportInProgress() && " is not needed in #78, due 
to #65.



repository/src/main/java/org/apache/atlas/repository/store/graph/v2/IDBasedEntityResolver.java
Lines 79 (patched)
<https://reviews.apache.org/r/75150/#comment315150>

    entityGraphMapper might be null due to #44. Either the constructor at #43 
should be removed or a null check must be added before #79.


- Madhan Neethiraj


On Aug. 16, 2024, 11:36 a.m., Priyanshi Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/75150/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2024, 11:36 a.m.)
> 
> 
> Review request for atlas, Jayendra Parab, Mandar Ambawane, Pinal Shah, and 
> Sheetal Shah.
> 
> 
> Bugs: ATLAS-4892
>     https://issues.apache.org/jira/browse/ATLAS-4892
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Problem:
> 
> During import, if shell entity is present in the importing file, import is 
> getting failed.
> 
> Solution:
> 
> There is mandatory attribute check present which was not allowing to create 
> shell entity and the import was getting failed due to this implementation.
> 
> 
> Diffs
> -----
> 
>   intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java 60e57a379 
>   intg/src/main/java/org/apache/atlas/type/AtlasStructType.java ffbe9e7d8 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityGraphDiscoveryV2.java
>  b51951fe2 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityStoreV2.java
>  656c9d14b 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
>  77ab99a96 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/IDBasedEntityResolver.java
>  2f37eac6b 
> 
> 
> Diff: https://reviews.apache.org/r/75150/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Priyanshi Shah
> 
>

Reply via email to