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