Re: Review Request 72477: ATLAS-3583 Use Audit framework to generate audit entries for TypeDefs CREATE, UPDATE and DELETE

2020-08-08 Thread Madhan Neethiraj 

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




repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 47 (patched)


atlasAuditService => auditService
  - avoid 'atlas' prefix in variable/member names



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 76 (patched)


add following condition as well:
  CollectionUtils.isNotEmpty(updatedTypes)



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 77 (patched)


typeNames => createdTypeNames



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 81 (patched)


Which use case results in duplicate entries in updatedTypes? If no 
duplicates are expected, simply return updatedTypes here. In fact, given 
'updatedTypes' is modified above (#79), consider not returning any value from 
this method.



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 90 (patched)


atlasBaseTypeDefList => typeDefs
  - avoid 'atlas' prefix in variable names



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 94 (patched)


clientId => clientIp



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 106 (patched)


Can you add details of the audit log generated? Computing 
groupByCategoryMap may not be useful; it should be enough to use following 
string as audit entry result:
  AtlasJson.toJson(typeDefs);
  
Audit entry params can be set to the list of typeNames.



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 108 (patched)


AtlasAuthorizationUtils.getCurrentUserName() => 
RequestContext.get().getUser()



repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java
Lines 725 (patched)


I suggest to create a PatchHandler for enum-def updates, instead of 
updating AddAttributePatchHandler:

  class UpdateEnumDefPatchHandler extends PatchHandler {
public UpdateEnumDefPatchHandler(AtlasTypeDefStore typeDefStore, 
AtlasTypeRegistry typeRegistry) {
  super(typeDefStore, typeRegistry, new String[] { "UPDATE_ENUMDEF" });
}

@Override
public PatchStatus applyPatch(TypeDefPatch patch) throws 
AtlasBaseException {
  ...
}
  }



repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java
Line 354 (original), 354 (patched)


CollectionUtils.size(obj) handles null value. Please review if following 
changes are necessary:
 - #354 - #359
 - #376 - #381


- Madhan Neethiraj


On Aug. 8, 2020, 2:42 p.m., Mandar Ambawane wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72477/
> ---
> 
> (Updated Aug. 8, 2020, 2:42 p.m.)
> 
> 
> Review request for atlas, Jayendra Parab, Madhan Neethiraj, Nixon Rodrigues, 
> Sarath Subramanian, and Sidharth Mishra.
> 
> 
> Bugs: ATLAS-3583
> https://issues.apache.org/jira/browse/ATLAS-3583
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> ATLAS-3583 Use Audit framework to generate audit entries for TypeDefs CREATE, 
> UPDATE and DELETE
> 
> 
> Diffs
> -
> 
>   
> addons/models/-Area0/patches/006-base_model_add_atlas_operation_attributes.json
>  PRE-CREATION 
>   intg/src/main/java/org/apache/atlas/model/audit/AtlasAuditEntry.java 
> a95cf4e 
>   intg/src/main/java/org/apache/atlas/model/audit/AuditSearchParameters.java 
> 9120062 
>   intg/src/test/java/org/apache/atlas/TestUtilsV2.java 2b9cf6e 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/AtlasAuditService.java
>  a0dc816 
>   
> repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
>  PRE-CREATION 
>   
> repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java
>  8e7c1b3 
>   
> repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java
>  79f5270 
>   
> repository/src/main/

Re: Review Request 72477: ATLAS-3583 Use Audit framework to generate audit entries for TypeDefs CREATE, UPDATE and DELETE

2020-08-08 Thread Mandar Ambawane 

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

(Updated Aug. 8, 2020, 2:42 p.m.)


Review request for atlas, Jayendra Parab, Madhan Neethiraj, Nixon Rodrigues, 
Sarath Subramanian, and Sidharth Mishra.


Changes
---

Test cases failing issue resolved.


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


Repository: atlas


Description
---

ATLAS-3583 Use Audit framework to generate audit entries for TypeDefs CREATE, 
UPDATE and DELETE


Diffs (updated)
-

  
addons/models/-Area0/patches/006-base_model_add_atlas_operation_attributes.json
 PRE-CREATION 
  intg/src/main/java/org/apache/atlas/model/audit/AtlasAuditEntry.java a95cf4e 
  intg/src/main/java/org/apache/atlas/model/audit/AuditSearchParameters.java 
9120062 
  intg/src/test/java/org/apache/atlas/TestUtilsV2.java 2b9cf6e 
  
repository/src/main/java/org/apache/atlas/repository/audit/AtlasAuditService.java
 a0dc816 
  
repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
 PRE-CREATION 
  
repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java
 8e7c1b3 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasTypeDefGraphStore.java
 79f5270 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasEntityChangeNotifier.java
 0dc3193 
  repository/src/test/java/org/apache/atlas/TestModules.java a298934 
  webapp/src/test/java/org/apache/atlas/web/adapters/TestTypeDefsREST.java 
PRE-CREATION 


Diff: https://reviews.apache.org/r/72477/diff/6/

Changes: https://reviews.apache.org/r/72477/diff/5-6/


Testing
---

Basic testing is done.

Pre-commit: 
https://builds.apache.org/job/PreCommit-ATLAS-Build-Test/1871/console

Pre-commit: 
https://builds.apache.org/job/PreCommit-ATLAS-Build-Test/1885/console

Pre-commit: 
https://builds.apache.org/job/PreCommit-ATLAS-Build-Test/1888/console

Pre-commit: 
https://builds.apache.org/job/PreCommit-ATLAS-Build-Test/2071/console

Pre-commit: 
https://builds.apache.org/job/PreCommit-ATLAS-Build-Test/2082/console


Thanks,

Mandar Ambawane