----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74109/#review224671 -----------------------------------------------------------
repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java Lines 478 (patched) <https://reviews.apache.org/r/74109/#comment313464> Use of HashSet here will eliminate duplicate rows. Is this by choice? Wouldn't this result in incorrect handling of pagination (offset)? repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java Lines 494 (patched) <https://reviews.apache.org/r/74109/#comment313463> Consider replacing #494 - #499 with Object value = entityRetriever.getVertexAttribute(vertex, attribute); i.e. use entityRetriever.getVertexAttribute() irrespective of the type of attribute - as it is generic enough to handle all types, including obj-ref/map/array/primitives. repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java Lines 507 (patched) <https://reviews.apache.org/r/74109/#comment313465> return here would result in skipping of scrubSearchResults() - which is incorrect. Please review and update. Instead of introducing #471 - #509, consider updating existing for loop below at #549, like: for (AtlasVertex atlasVertex : resultList) { if (searchContext.excludeHeaderAttributes()) { // code from #475 - #506 } else { // existing code ... } } repository/src/main/java/org/apache/atlas/discovery/SearchContext.java Lines 356 (patched) <https://reviews.apache.org/r/74109/#comment313466> Validations in this if block seem to be valid irrespective of whether excludeHeaderAttributes is set or not. If yes, consider removing #356 and #370. repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java Line 1356 (original), 1356 (patched) <https://reviews.apache.org/r/74109/#comment313467> Consider retaining getVertexAttribute() as private, based on comment in EntityDiscoveryService.java #494. - Madhan Neethiraj On Sept. 14, 2022, 12:11 p.m., Pinal Shah wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74109/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2022, 12:11 p.m.) > > > Review request for atlas, Jayendra Parab, Radhika Kundam, and Sidharth Mishra. > > > Bugs: ATLAS-4671 > https://issues.apache.org/jira/browse/ATLAS-4671 > > > Repository: atlas > > > Description > ------- > > **Issue** : Basic search has AtlasEntityHeader of each entity in the response. > AtlasEntityHeader has many attributes including classification and terms. > hence for each attribute, it will request janusgraph > > **Approach** : To overcome, we can add a flag to exclude other attributes and > add only selected attributes from 'attributes' field in the response > > **Request**: { > "excludeDeletedEntities": true, > "includeSubClassifications": true, > "includeSubTypes": true, > "includeClassificationAttributes": true, > "limit": 25, > "offset": 0, > "typeName": "hdfs_path", > "attributes": ["path", "name"], > "excludeHeaderAttributes": "true" > } > > **Response**: { > "queryType": "BASIC", > "searchParameters": { > "typeName": "hdfs_path", > "excludeDeletedEntities": true, > "includeClassificationAttributes": true, > "includeSubTypes": true, > "includeSubClassifications": true, > "limit": 25, > "offset": 0, > "attributes": ["path", "name"] > }, > "attributes": { > "name": ["path", "name"], > "values": [ > ["/data/warehouse/customer", "customer"], > ["/data/warehouse/sales", "sales"] > ] > }, > "approximateCount": 2 > } > > > Diffs > ----- > > > intg/src/main/java/org/apache/atlas/model/discovery/QuickSearchParameters.java > 79f5aae0d > intg/src/main/java/org/apache/atlas/model/discovery/SearchParameters.java > 78fb4a48f > > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java > 8fbc22fa0 > repository/src/main/java/org/apache/atlas/discovery/SearchContext.java > 01954d07e > > repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java > a8fe5a762 > > repository/src/test/java/org/apache/atlas/discovery/AtlasDiscoveryServiceTest.java > fbc739652 > > > Diff: https://reviews.apache.org/r/74109/diff/4/ > > > Testing > ------- > > Unit testcases added > Precommit : > https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/1215/ > > Performance Readings: > Basic Search on 20K entities, limit 1000 > > > Thanks, > > Pinal Shah > >
