-----------------------------------------------------------
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)
<https://reviews.apache.org/r/72477/#comment310558>

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



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 76 (patched)
<https://reviews.apache.org/r/72477/#comment310554>

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



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 77 (patched)
<https://reviews.apache.org/r/72477/#comment310555>

    typeNames => createdTypeNames



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 81 (patched)
<https://reviews.apache.org/r/72477/#comment310556>

    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)
<https://reviews.apache.org/r/72477/#comment310557>

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



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 94 (patched)
<https://reviews.apache.org/r/72477/#comment310560>

    clientId => clientIp



repository/src/main/java/org/apache/atlas/repository/audit/TypeDefAuditListener.java
Lines 106 (patched)
<https://reviews.apache.org/r/72477/#comment310561>

    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)
<https://reviews.apache.org/r/72477/#comment310559>

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



repository/src/main/java/org/apache/atlas/repository/store/bootstrap/AtlasTypeDefStoreInitializer.java
Lines 725 (patched)
<https://reviews.apache.org/r/72477/#comment310552>

    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)
<https://reviews.apache.org/r/72477/#comment310553>

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

Reply via email to