----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72270/#review220249 -----------------------------------------------------------
intg/src/main/java/org/apache/atlas/listener/EntityChangeListenerV2.java Lines 178 (patched) <https://reviews.apache.org/r/72270/#comment308581> consider adding comments to be in alignment with other methods in this interface. intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java Lines 54 (patched) <https://reviews.apache.org/r/72270/#comment308582> BUSINESS_ATTRIBUTE_ADD => BUSINESS_METADATA_ATTRIBUTE_ADD BUSINESS_ATTRIBUTE_UPDATE => BUSINESS_METADATA_ATTRIBUTE_UPDATE BUSINESS_ATTRIBUTE_DELETE => BUSINESS_METADATA_ATTRIBUTE_DELETE consider updating in line 95, 97 and 99 as well repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityChangeNotifier.java Lines 411 (patched) <https://reviews.apache.org/r/72270/#comment308587> why this check is added? we are not creating any audit vertex for business metadata add/update/delete. Please review and remove. repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java Lines 452 (patched) <https://reviews.apache.org/r/72270/#comment308584> instead of creating audit entries for every iteration of for loop. Consider sending audits at the end to avoid multiple calls to write to audit repository. maintain a list of business metadata attributes - added/updated/deleted for entity and send audit notification outside for loop. repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java Lines 520 (patched) <https://reviews.apache.org/r/72270/#comment308585> same comment. consider sending audit entries at the end - outside for loop. repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java Lines 567 (patched) <https://reviews.apache.org/r/72270/#comment308583> consider adding the audit entry after you exit out of for loop, so notification is sent at the end. A recent change was added to add bulk audit notification for add classifications. Please review. webapp/src/main/java/org/apache/atlas/notification/EntityNotificationListenerV2.java Lines 323 (patched) <https://reviews.apache.org/r/72270/#comment308586> nit: add comments inside these methods - "// do nothing -> notification not sent out for business metadata attribute addition/updation/removal from entities" - Sarath Subramanian On April 6, 2020, 11:21 p.m., Mandar Ambawane wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72270/ > ----------------------------------------------------------- > > (Updated April 6, 2020, 11:21 p.m.) > > > Review request for atlas, Ashutosh Mestry, Jayendra Parab, Madhan Neethiraj, > Nixon Rodrigues, Sarath Subramanian, and Sidharth Mishra. > > > Bugs: ATLAS-3689 > https://issues.apache.org/jira/browse/ATLAS-3689 > > > Repository: atlas > > > Description > ------- > > Add audits entries when Namespace Attributes are added/updated/deleted to an > entity > > > Diffs > ----- > > intg/src/main/java/org/apache/atlas/listener/EntityChangeListenerV2.java > 2394a12 > intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java > 9301e21 > > repository/src/main/java/org/apache/atlas/repository/audit/AtlasAuditService.java > 590f7a0 > > repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java > cab4e1e > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityChangeNotifier.java > 00c0114 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java > 75b016c > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/IAtlasEntityChangeNotifier.java > c4dc5a1 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/bulkimport/EntityChangeNotifierNop.java > 2943ea9 > > webapp/src/main/java/org/apache/atlas/notification/EntityNotificationListenerV2.java > 6d64fec > > > Diff: https://reviews.apache.org/r/72270/diff/2/ > > > Testing > ------- > > Pre-commit: > https://builds.apache.org/job/PreCommit-ATLAS-Build-Test/1759/console > > Basic testing is done. > > > Thanks, > > Mandar Ambawane > >
