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