Re: Review Request 66797: ATLAS-2609: Update audit listener to handle propagated classification add/delete/update

2018-04-25 Thread Sarath Subramanian 

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

(Updated April 25, 2018, 9:39 p.m.)


Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.


Changes
---

addressed review comments


Bugs: ATLAS-2609
https://issues.apache.org/jira/browse/ATLAS-2609


Repository: atlas


Description
---

Currently audit are not generated for propagated classification add/update or 
delete.

For Audit:
* Adding Propagated Classification   => Send 'Propagated Classifications Added' 
message to audit repository. 
* Removing Propagated Classification => Send 'Propagated Classifications 
Deleted' message to audit repository.
* Updating Propagated Classification => Send 'Propagated Classifications 
Updated' message to audit repository.


Diffs (updated)
-

  client/client-v1/src/main/java/org/apache/atlas/EntityAuditEvent.java 
3cf3e212 
  dashboardv2/public/js/utils/Enums.js 8c0518bf 
  intg/src/main/java/org/apache/atlas/listener/EntityChangeListenerV2.java 
70877d22 
  intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java 
1045c774 
  
repository/src/main/java/org/apache/atlas/repository/audit/AbstractStorageBasedAuditRepository.java
 af6c4e62 
  
repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
 1c04eea4 
  
repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java
 bb510149 
  
repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java
 c5967b27 
  
repository/src/main/java/org/apache/atlas/repository/converters/AtlasInstanceConverter.java
 20147a0c 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java
 9aebde2b 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
 51dcc3a5 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
 c0f2fc3d 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
 43e67df7 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
 7d5b7fb6 
  
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java
 8fc0327a 
  server-api/src/main/java/org/apache/atlas/RequestContextV1.java b7c56865 
  server-api/src/main/java/org/apache/atlas/listener/EntityChangeListener.java 
19fae4fd 
  
tools/atlas-migration-exporter/src/main/java/org/apache/atlas/migration/NoOpNotificationChangeListener.java
 6c2bf220 
  
webapp/src/main/java/org/apache/atlas/notification/EntityNotificationListenerV2.java
 ac097b60 
  
webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java
 a3e5949c 


Diff: https://reviews.apache.org/r/66797/diff/3/

Changes: https://reviews.apache.org/r/66797/diff/2-3/


Testing
---

Precommit validated: 
https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/307/console

Validated audit/notifications manually for propagated classification 
add/update/delete


Thanks,

Sarath Subramanian

Re: Review Request 66797: ATLAS-2609: Update audit listener to handle propagated classification add/delete/update

2018-04-25 Thread Madhan Neethiraj 

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


Fix it, then Ship it!





repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java
Lines 148 (patched)


Use of isPropagatedClassificationUpdated can be avoided by having #148 - 
#150 as 'else' block at line #146.



repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java
Line 138 (original), 164 (patched)


Consider replacing 
"entity.getGuid().equals(classification.getEntityGuid())" with the following:

  StringUtils.equals(entity.getGuid(), classification.getEntityGuid())



repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java
Lines 234 (patched)


Consider replacing "equals()" with StringUtil.equals().



server-api/src/main/java/org/apache/atlas/RequestContextV1.java
Lines 110 (patched)


recordAddPropagation() ==> recordAddedPropagation()



server-api/src/main/java/org/apache/atlas/RequestContextV1.java
Lines 124 (patched)


recordRemovePropagation() ==> recordRemovedPropagation()


- Madhan Neethiraj


On April 25, 2018, 8:29 p.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66797/
> ---
> 
> (Updated April 25, 2018, 8:29 p.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2609
> https://issues.apache.org/jira/browse/ATLAS-2609
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Currently audit are not generated for propagated classification add/update or 
> delete.
> 
> For Audit:
> * Adding Propagated Classification   => Send 'Propagated Classifications 
> Added' message to audit repository. 
> * Removing Propagated Classification => Send 'Propagated Classifications 
> Deleted' message to audit repository.
> * Updating Propagated Classification => Send 'Propagated Classifications 
> Updated' message to audit repository.
> 
> 
> Diffs
> -
> 
>   client/client-v1/src/main/java/org/apache/atlas/EntityAuditEvent.java 
> 3cf3e212 
>   dashboardv2/public/js/utils/Enums.js 8c0518bf 
>   intg/src/main/java/org/apache/atlas/listener/EntityChangeListenerV2.java 
> 70877d22 
>   intg/src/main/java/org/apache/atlas/model/audit/EntityAuditEventV2.java 
> 1045c774 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/AbstractStorageBasedAuditRepository.java
>  af6c4e62 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListener.java
>  1c04eea4 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java
>  bb510149 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java
>  c5967b27 
>   
> repository/src/main/java/org/apache/atlas/repository/converters/AtlasInstanceConverter.java
>  20147a0c 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java
>  9aebde2b 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
>  51dcc3a5 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
>  c0f2fc3d 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
>  4225a80d 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
>  7d5b7fb6 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java
>  8fc0327a 
>   server-api/src/main/java/org/apache/atlas/RequestContextV1.java b7c56865 
>   
> server-api/src/main/java/org/apache/atlas/listener/EntityChangeListener.java 
> 19fae4fd 
>   
> tools/atlas-migration-exporter/src/main/java/org/apache/atlas/migration/NoOpNotificationChangeListener.java
>  6c2bf220 
>   
> webapp/src/main/java/org/apache/atlas/notification/EntityNotificationListenerV2.java
>  ac097b60 
>   
> webapp/src/main/java/org/apache/atlas/notification/NotificationEntityChangeListener.java
>  a3e5949c 
> 
> 
> Diff: https://reviews.apache.org/r/66797/diff/2/
> 
> 
> Testing
> ---
> 
> Precommit validated: 
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/307/console
> 
> Validated audit/notifications manually for propagated classification 
> add/update/delete
> 
> 
> Thanks,
> 
> Sarath Subramanian
> 
>