----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62342/#review185646 -----------------------------------------------------------
client/src/main/java/org/apache/atlas/EntityAuditEventV2.java Lines 29 (patched) <https://reviews.apache.org/r/62342/#comment261963> If this class is going to be exposed via REST API, please move to org.apache.atlas.model package. client/src/main/java/org/apache/atlas/EntityAuditEventV2.java Lines 31 (patched) <https://reviews.apache.org/r/62342/#comment261968> Is it necessary to store entire entity here, given that 'details' will include all the information to be saved in audit? notification/src/main/java/org/apache/atlas/notification/entity/EntityNotificationV2.java Lines 91 (patched) <https://reviews.apache.org/r/62342/#comment261967> Please review for the following cases: - on ENTITY_CREATE, the notification would have to include propagatedTags as well - on ENTITY_DELETE, the notification should include the classifications & list of impacted entities (from whom the classification should be removed) - how are create/delete of edges handled in notificaitons? They would impact the propagatedTags for other entities. repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java Lines 129 (patched) <https://reviews.apache.org/r/62342/#comment261972> It might be efficient to create a single event that includes all classifications - instead of creating one event for each classfication. Please review classificationsUpdated() and classificationsDeleted() as well. repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java Lines 207 (patched) <https://reviews.apache.org/r/62342/#comment261970> This may not work, as clearAttributeValues() call above (line #203) would infact set all entries in 'attrValues' (line #201). Please review. repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java Lines 187 (patched) <https://reviews.apache.org/r/62342/#comment261974> For large entities, wouldn't this cause failure in HBase save? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java Line 94 (original), 102 (patched) <https://reviews.apache.org/r/62342/#comment261975> FullTextMapperV2.java uses RequestContext to get the entity from cache, if it exists. It will be efficient than loading entities from the store again. Please review FullTextMapperV2.getAndCacheEntity(guid) - Madhan Neethiraj On Sept. 19, 2017, 3 a.m., Sarath Subramanian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62342/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2017, 3 a.m.) > > > Review request for atlas and Madhan Neethiraj. > > > Bugs: ATLAS-2136 > https://issues.apache.org/jira/browse/ATLAS-2136 > > > Repository: atlas > > > Description > ------- > > Currently audit and notification entityChangeListeners is using v1 API. This > needs to be migrated to start using v2 APIs. This is needed for notification > of tag propagation. > Also provide a new REST API to get audit events for an entity using v2 > structure > > > Diffs > ----- > > client/src/main/java/org/apache/atlas/EntityAuditEventV2.java PRE-CREATION > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java d205faf5 > > notification/src/main/java/org/apache/atlas/notification/entity/EntityNotificationV2.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditListenerV2.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/audit/EntityAuditRepository.java > 9dc78350 > > repository/src/main/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepository.java > 5a5a2c10 > > repository/src/main/java/org/apache/atlas/repository/audit/InMemoryEntityAuditRepository.java > 22d2a810 > > repository/src/main/java/org/apache/atlas/repository/audit/NoopEntityAuditRepository.java > c3826019 > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java > 5a0b74e3 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java > 7b349c46 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java > a5db81bf > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java > f6aa7bbf > > repository/src/test/java/org/apache/atlas/repository/audit/AuditRepositoryV2TestBase.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportServiceTest.java > 8eb7a510 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java > 8d040946 > > repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java > 94cc5b93 > > server-api/src/main/java/org/apache/atlas/listener/EntityChangeListener.java > e05a7755 > > server-api/src/main/java/org/apache/atlas/listener/EntityChangeListenerV2.java > PRE-CREATION > > webapp/src/main/java/org/apache/atlas/notification/EntityNotificationListenerV2.java > PRE-CREATION > webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java 0f6eeb11 > > > Diff: https://reviews.apache.org/r/62342/diff/3/ > > > Testing > ------- > > Validated HBase audit events and Kafka notification message for the following > operations: > 1. Entity Create > 2. Entity Update > 3. Entity Delete > 4. Add Classification > 5. Delete Classification > 6. Update Classification > > Listeners were notified using v2 API structure > > > Thanks, > > Sarath Subramanian > >
