----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71791/#review219365 -----------------------------------------------------------
addons/models/0000-Area0/0010-base_model.json Lines 4 (patched) <https://reviews.apache.org/r/71791/#comment307512> if this enum is used only for aud consider renaming to atlas_audit_operation addons/models/0000-Area0/0010-base_model.json Lines 5 (patched) <https://reviews.apache.org/r/71791/#comment307511> => "Defines audit operations in Atlas" client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 76 (patched) <https://reviews.apache.org/r/71791/#comment307518> "audit/" => "audits/" client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 93 (patched) <https://reviews.apache.org/r/71791/#comment307519> unused variables - 93-98. Consider removing them. client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java Lines 495 (patched) <https://reviews.apache.org/r/71791/#comment307520> why return type is ArrayNode.class? can we not have return type to be - List<AtlasAuditEntry> ? if ti works we don't need ExtractOperation() and ExtractResults() intg/src/main/java/org/apache/atlas/model/audit/AtlasAuditEntry.java Lines 45 (patched) <https://reviews.apache.org/r/71791/#comment307514> fix alignment in line 45-51 and 57-59. Extra spaces intg/src/main/java/org/apache/atlas/model/audit/AuditSearchParameters.java Lines 36 (patched) <https://reviews.apache.org/r/71791/#comment307521> AuditSearchParameters class looks like a trimmed down version of SearchParameters class. Consider using SearchParameters instead and removeA uditSearchParameters class. repository/src/main/java/org/apache/atlas/repository/ogm/AtlasAuditEntryDTO.java Lines 38 (patched) <https://reviews.apache.org/r/71791/#comment307516> nit: fix alignment (extra spaces) - line 38-44 repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java Lines 1053 (patched) <https://reviews.apache.org/r/71791/#comment307522> move line 1053 after 1055 repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java Lines 1059 (patched) <https://reviews.apache.org/r/71791/#comment307523> refactor line 1059 and 1060 to for better readability: if (reqContext.isPurgeRequested()) { ret = vertexStatus == ACTIVE; // skip purging ACTIVE vertices } else { ret = vertexStatus == DELETED; // skip deleting DELETED vertices } repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java Line 85 (original), 86 (patched) <https://reviews.apache.org/r/71791/#comment307524> why ImportServiceTest now need to extend from AtlasAuditTestBase? AtlasAuditTestBase should be specific to only atlas sudit tests. Move common methods to a new base class - AtlasBaseTest. Please review all tests which is modified now to extend from 'AtlasAuditTestBase' webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java Lines 574 (patched) <https://reviews.apache.org/r/71791/#comment307517> "/audit" => "/audits" webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java Lines 577 (patched) <https://reviews.apache.org/r/71791/#comment307513> getAtlasAudit => getAtlasAudits; update line 582 as well. - Sarath Subramanian On Dec. 10, 2019, 5:17 p.m., Sidharth Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71791/ > ----------------------------------------------------------- > > (Updated Dec. 10, 2019, 5:17 p.m.) > > > Review request for atlas, Ashutosh Mestry, Madhan Neethiraj, and Sarath > Subramanian. > > > Bugs: ATLAS-3518 > https://issues.apache.org/jira/browse/ATLAS-3518 > > > Repository: atlas > > > Description > ------- > > Right now for purge, delete entity, etc. at Atlas we create audit entry at > HBase. User can go to entity and click on audit to see the audit information > for that particular entity. But if user purge one entity there will not be > any vertex at Janus graph and user will not able to get the purge audit which > is important for governance purpose. > > As part of fix, implemented Atlas Audit Framework which is being used for > Purge operation and can be extended for other operations. In Future once we > change the AtlasAudit type as super type for Import export as well we can use > this framework for all Atlas operations. > > admin/audit rest has been added which will return all the Atlas audit > operations including import export at Atlas. > > > Diffs > ----- > > addons/models/0000-Area0/0010-base_model.json 2f5fdaf14 > client/client-v2/src/main/java/org/apache/atlas/AtlasClientV2.java > 8c0a6407c > dashboardv2/public/js/utils/Enums.js a7de0d451 > dashboardv3/public/js/utils/Enums.js a7de0d451 > intg/src/main/java/org/apache/atlas/model/audit/AtlasAuditEntry.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/model/audit/AuditSearchParameters.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/instance/EntityMutationResponse.java > b448d510b > > repository/src/main/java/org/apache/atlas/repository/audit/AtlasAuditService.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/ogm/AtlasAuditEntryDTO.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java > c9ed79750 > repository/src/test/java/org/apache/atlas/TestModules.java 94b08a187 > repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java > be1698fb4 > repository/src/test/java/org/apache/atlas/query/BasicTestSetup.java > 02f78b369 > > repository/src/test/java/org/apache/atlas/repository/audit/AdminPurgeTest.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/repository/audit/AtlasAuditServiceTest.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/repository/impexp/AtlasServerServiceTest.java > 78865c569 > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportImportAuditServiceTest.java > ba7a8a04b > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportImportTestBase.java > 925b2a007 > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportIncrementalTest.java > 4d438525a > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportServiceTest.java > db8c6c83c > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportSkipLineageTest.java > 25e0a5362 > > repository/src/test/java/org/apache/atlas/repository/impexp/HdfsPathEntityCreatorTest.java > 1863b8d74 > > repository/src/test/java/org/apache/atlas/repository/impexp/ImportReactivateTableTest.java > d0c06a107 > > repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java > c14850f43 > > repository/src/test/java/org/apache/atlas/repository/impexp/ImportTransformsShaperTest.java > 78fdaca8d > > repository/src/test/java/org/apache/atlas/repository/impexp/IncrementalExportEntityProviderTest.java > 42b63535b > > repository/src/test/java/org/apache/atlas/repository/impexp/RelationshipAttributesExtractorTest.java > a1b512fb9 > > repository/src/test/java/org/apache/atlas/repository/impexp/ReplicationEntityAttributeTest.java > 7a1ed18cb > > repository/src/test/java/org/apache/atlas/repository/impexp/StartEntityFetchByExportRequestTest.java > 48b958271 > > repository/src/test/java/org/apache/atlas/repository/impexp/TableReplicationRequestProcessorTest.java > c9bb11c56 > > repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java > 0ffc3d595 > > repository/src/test/java/org/apache/atlas/repository/migration/MigrationBaseAsserts.java > 18e950e8c > > repository/src/test/java/org/apache/atlas/repository/migration/RelationshipCacheGeneratorTest.java > 050bda3ae > > repository/src/test/java/org/apache/atlas/repository/tagpropagation/ClassificationPropagationTest.java > 6f9c05e7a > > repository/src/test/java/org/apache/atlas/repository/userprofile/UserProfileServiceTest.java > e56e48308 > repository/src/test/java/org/apache/atlas/services/MetricsServiceTest.java > baeafd4b7 > repository/src/test/java/org/apache/atlas/utils/TestLoadModelUtils.java > PRE-CREATION > > repository/src/test/resources/json/auditSearchParameters/audit-search-parameter-purge.json > PRE-CREATION > > repository/src/test/resources/json/auditSearchParameters/audit-search-parameter-without-filter.json > PRE-CREATION > server-api/src/main/java/org/apache/atlas/RequestContext.java f9ca7a26d > webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java > 40fd6efdc > webapp/src/test/java/org/apache/atlas/web/adapters/TestEntitiesREST.java > afc4c50d7 > > webapp/src/test/java/org/apache/atlas/web/integration/EntityV2JerseyResourceIT.java > a4b7b0d49 > webapp/src/test/java/org/apache/atlas/web/resources/AdminResourceTest.java > 563a16f64 > webapp/src/test/resources/json/audit-search-parameter-purge.json > PRE-CREATION > > > Diff: https://reviews.apache.org/r/71791/diff/13/ > > > Testing > ------- > > Two UT and one IT has been added to test functionality like: > 1. Atlas Audit entry save and retrive > 2. Atlas Purge Audit store, retrive and search (UT and IT) > > > Thanks, > > Sidharth Mishra > >
