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




repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java
Lines 50 (patched)
<https://reviews.apache.org/r/68427/#comment291200>

    Inline with tags we use in demo, use upper-case letters for tag names, like 
"REPLICATED".



repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java
Lines 63 (patched)
<https://reviews.apache.org/r/68427/#comment291201>

    AuditsWriter doesn't seem to be the right place for 'entityTagger'. 
Treating it as a transformer (during import) might be a better choice. Please 
review.



repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java
Line 169 (original), 176 (patched)
<https://reviews.apache.org/r/68427/#comment291202>

    ImportAudits doesn't seem to be the right place for 
'updateReplicationAttributeForImport()'. Treating it as a transformer (during 
import) might be a better choice. Please review.



repository/src/main/java/org/apache/atlas/repository/impexp/EntityTagger.java
Lines 40 (patched)
<https://reviews.apache.org/r/68427/#comment291204>

    Consider marking these as 'final'



repository/src/main/java/org/apache/atlas/repository/impexp/EntityTagger.java
Lines 62 (patched)
<https://reviews.apache.org/r/68427/#comment291203>

    Is 'operation' neded here? line #52 passes the classification name for this 
param. Consider removing it.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java
Lines 541 (patched)
<https://reviews.apache.org/r/68427/#comment291207>

    Instead of try/catch to detect if the entity is already associated with 
this tag, consider refactoring validateEntityAssociations(), like:
    
    if (hasClassification(guid, classification)) {
      if (!ignoreIfAlreadyPresent) {
        throw new AtlasBaseException(AtlasErrorCode.INVALID_PARAMETERS, 
"entity: " + guid + ", is already associated with classification: "  + 
newClassification);
      }
    }


- Madhan Neethiraj


On Aug. 21, 2018, 4:48 p.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68427/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2018, 4:48 p.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Madhan Neethiraj, and Sarath 
> Subramanian.
> 
> 
> Bugs: ATLAS-2818
>     https://issues.apache.org/jira/browse/ATLAS-2818
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> **Approach**
> - New method added to _AtlasEntityStore_ to add classifications without 
> updating entity metadata.
> - New class _EntityTagger_ added.
> - Modified _AudisWriter.ImportAudit_ to include call to _EntityTagger_.
> 
> 
> Diffs
> -----
> 
>   
> repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java 
> 467d3839a4aff71b7f3d1e03ccd0a828d2ce8c76 
>   
> repository/src/main/java/org/apache/atlas/repository/impexp/ClusterService.java
>  950850e92640fd427d68b8ab1bee5a8f810119cd 
>   
> repository/src/main/java/org/apache/atlas/repository/impexp/EntityTagger.java 
> PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java
>  6a1a88f4fc714ef069b38cdffd2c5fcc653869e1 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java
>  2bfef788d1ff185c635879f329fa62daafec45ce 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/BulkImporterImpl.java
>  467ced7748e934b4829ee6f7bbafac39020529ac 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
>  47960f3f760c322d250559fa1cee2e78b8de2d52 
>   
> repository/src/test/java/org/apache/atlas/repository/impexp/EntityTaggerTest.java
>  PRE-CREATION 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1BulkImportPercentTest.java
>  73dfe37c786336ec267247a716a27f53e0d74993 
> 
> 
> Diff: https://reviews.apache.org/r/68427/diff/4/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> New tests added.
> 
> **Pre-commit**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/598/
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>

Reply via email to