----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68270/#review207210 -----------------------------------------------------------
intg/src/main/java/org/apache/atlas/model/clusterinfo/AtlasCluster.java Lines 80 (patched) <https://reviews.apache.org/r/68270/#comment290504> line #80 is not necessary, as this will be handled in #89 below. intg/src/main/java/org/apache/atlas/model/clusterinfo/AtlasCluster.java Lines 85 (patched) <https://reviews.apache.org/r/68270/#comment290505> Use of stringified-AtlasObjectId as key doesn't sound good - since AtlasObjectId has guid/type/state/uniqueAttributes. Why not simply use entiy guid as the key? intg/src/main/java/org/apache/atlas/model/impexp/ReplicationDetails.java Lines 37 (patched) <https://reviews.apache.org/r/68270/#comment290506> Why is it necessary to store operation & lastUpdate values? Only lastModifiedTimestamp should be enough, right? I suggest to avoid ReplicationDetails class completely and instead have the caller: - retrieve AtlasCluster - read additionalInfo.get("REPL_STATUS").get(entityGuid) <== this should return the modifiedTime of the entity during previous repl-import In addition, remove AdminResource.getReplicationDetails() and AtlasBaseClient.getReplicationDetails() repository/src/main/java/org/apache/atlas/repository/impexp/ExportImportAuditService.java Lines 83 (patched) <https://reviews.apache.org/r/68270/#comment290502> "result.getEntities() == null" - perhaps you meant "result == null"? repository/src/main/java/org/apache/atlas/repository/impexp/ExportImportAuditService.java Lines 88 (patched) <https://reviews.apache.org/r/68270/#comment290503> Instead of this 'for' loop to retrieve each entity from db, it will be efficient to covert entityHeader (AtlasEntityHeader) into ExportImportAuditEntry object. Search API can return necessary attributes (to populate ExportImportAuditEntry); just update searchParameters in #77 to include list of attributes: searchParameters.setAttribute(attributeNames). repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java Lines 115 (patched) <https://reviews.apache.org/r/68270/#comment290501> Why pay the cost of AtlasType.toJson() call, when debug is not enabled? Replace lines #115 - #121 with the following: if (LOG.isDebugEnabled()) { LOG.debug(" => transforms: {}", AtlasType.toJson(importTransform)); } Same for such usage in ExportService.java as well. - Madhan Neethiraj On Aug. 13, 2018, 7:07 p.m., Ashutosh Mestry wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68270/ > ----------------------------------------------------------- > > (Updated Aug. 13, 2018, 7:07 p.m.) > > > Review request for atlas, Apoorv Naik, Madhan Neethiraj, and Sarath > Subramanian. > > > Bugs: ATLAS-2814 > https://issues.apache.org/jira/browse/ATLAS-2814 > > > Repository: atlas > > > Description > ------- > > **Approach** > - New model _ReplicationDetails_ store replication timestamp. > - _AuditWriter_ updates appropriate _AtlasCluster_ entity with > _ReplicationDetails_. > > **REST Call** > Endpoint: /admin/expimp/audit > CURL: > curl -X GET -u admin:admin -H "Content-Type: application/json" -H > "Cache-Control: no-cache" > http://localhost:21000/api/atlas/admin/expimp/audit?sourceClusterName=cl2 > > > Diffs > ----- > > client/common/src/main/java/org/apache/atlas/AtlasBaseClient.java > f73ba2c6e6c9d05e86eb6c17f923a6dda1df5dd9 > intg/src/main/java/org/apache/atlas/model/clusterinfo/AtlasCluster.java > 3ce50e38b3af45521c2c83d0e6c05398747bcf86 > intg/src/main/java/org/apache/atlas/model/impexp/AtlasExportResult.java > 85a606c7d3b2158238b2a6defbb9185883434d78 > intg/src/main/java/org/apache/atlas/model/impexp/AtlasImportResult.java > bfb7637711e08e17fd5f5b16bf121ac541b631a4 > intg/src/main/java/org/apache/atlas/model/impexp/ReplicationDetails.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/impexp/AuditsWriter.java > 6a3fbecda84dc48e0df1359c1b5a248958db0a4c > > repository/src/main/java/org/apache/atlas/repository/impexp/ClusterService.java > fd8e2bfe6bbdb7ae5e66ab67ecdf94e9527d9b09 > > repository/src/main/java/org/apache/atlas/repository/impexp/ExportImportAuditService.java > e90b6b942d359f78ccf59ee955c4667f2033e6c4 > > repository/src/main/java/org/apache/atlas/repository/impexp/ExportService.java > b15f828e470eaee972a4ee3f8447f404f813c699 > > repository/src/main/java/org/apache/atlas/repository/impexp/ImportService.java > 98ef389bd1bf8f87d23a06dc231d4d93f06b2231 > > repository/src/main/java/org/apache/atlas/repository/ogm/AtlasClusterDTO.java > 424fb88a5ad2d84ac2245848d061892d40a3f50c > repository/src/main/java/org/apache/atlas/repository/ogm/DataAccess.java > b7e943f14084882cd5a3ad22c21168ea098eefcb > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java > 9e7a119a3abfb50401f9747dad1ffa55facc51ed > > repository/src/test/java/org/apache/atlas/repository/impexp/ClusterServiceTest.java > cfd272fb00a970ab0ba28f96b237de6bd5c694e1 > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportImportAuditServiceTest.java > f3803e5ae15a772eb4ff04192a83a5b412631791 > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportImportTestBase.java > fcf90d364950d3afa2c18689d8e22616cfcd3c19 > > repository/src/test/java/org/apache/atlas/repository/impexp/ExportIncrementalTest.java > 86ab22294918701270db3ba849f5b0532c1a21a4 > > repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java > dc25e92d7115db31cbd7f99bd1a2cb5f75e766a5 > > repository/src/test/java/org/apache/atlas/repository/impexp/ReplicationEntityAttributeTest.java > 881368ce2c6803c8cf887a1fc91737c045c5e663 > server-api/src/main/java/org/apache/atlas/RequestContextV1.java > 8506d186978f579c6b18c4347fc7cb2636848f16 > webapp/src/main/java/org/apache/atlas/web/resources/AdminResource.java > d4e72620b15f69d258e5a2a96521c65a7ef71114 > > webapp/src/test/java/org/apache/atlas/web/resources/AdminExportImportTestIT.java > fc804d2e5a1debd7e728dca1773f91402d41fc26 > webapp/src/test/resources/json/export-incremental.json > 97108411f9e0c177ac13eb0a8ec3bd4475499f89 > webapp/src/test/resources/stocks-base.zip > 40c7f37eefb46a944921f6a74a916191704cb9a3 > > > Diff: https://reviews.apache.org/r/68270/diff/5/ > > > Testing > ------- > > **Unit tests** > New tests added. > Unit tests related to audits now pause for 5 secs before performing asserts. > This should give time for indexes to be created. > > > Thanks, > > Ashutosh Mestry > >
