----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66518/#review200781 -----------------------------------------------------------
addons/models/0000-Area0/0011-glossary_model.json Lines 100 (patched) <https://reviews.apache.org/r/66518/#comment281648> Please set isIndexable=true, to enable search glossary by displayName. addons/models/0000-Area0/0011-glossary_model.json Lines 157 (patched) <https://reviews.apache.org/r/66518/#comment281649> Please set isIndexable=true, to enable search term by displayName. addons/models/0000-Area0/0011-glossary_model.json Lines 222 (patched) <https://reviews.apache.org/r/66518/#comment281650> Please set isIndexable=true, to enable search category by displayName. addons/models/0000-Area0/0011-glossary_model.json Lines 260 (patched) <https://reviews.apache.org/r/66518/#comment281651> Not sure if indexing is necessary for relationship attributes. Please review and update. addons/models/0000-Area0/0011-glossary_model.json Lines 333 (patched) <https://reviews.apache.org/r/66518/#comment281652> If 'isLegacyAttribute' is not needed, consider removing it from model json. graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/tinkerpop/query/TinkerpopGraphQuery.java Lines 276 (patched) <https://reviews.apache.org/r/66518/#comment281653> "final" in method argument seems unncessary - in most cases. Any reason this quailifer is added here, and a lot of other methods added in this patch? intg/src/main/java/org/apache/atlas/model/instance/AtlasEntityHeader.java Lines 61 (patched) <https://reviews.apache.org/r/66518/#comment281655> Consider renaming the following, to be aligned with the attribute name in AtlasEntity: "termNames" ==> "meaningNames" "terms" ==> "meanings" repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AbstractGlossaryDTO.java Lines 136 (patched) <https://reviews.apache.org/r/66518/#comment281670> '(String) status' would fail as the previous line checks for status to be of type AtlasTermRelationshipStatus. repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryCategoryDTO.java Lines 131 (patched) <https://reviews.apache.org/r/66518/#comment281669> relationship attributes 'anchor' 'parentCategory', 'childrenCategories' and 'terms' are not set in ret? repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryDTO.java Lines 127 (patched) <https://reviews.apache.org/r/66518/#comment281668> relationship attributes 'terms' and 'categories' are not set in ret? repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryDTO.java Lines 141 (patched) <https://reviews.apache.org/r/66518/#comment281667> Shouldn't all related entities (like terms, categories) be added to 'ret' as 'referredEntities'? repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryTermDTO.java Lines 236 (patched) <https://reviews.apache.org/r/66518/#comment281665> Shouldn't all related entities be added to 'ret' as 'referredEntities'? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java Lines 369 (patched) <https://reviews.apache.org/r/66518/#comment281664> Address this 'TODO' webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java Lines 216 (patched) <https://reviews.apache.org/r/66518/#comment281663> Move this method to line #57, along with rest of methods that deal with AtlasGlossary object. webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java Lines 597 (patched) <https://reviews.apache.org/r/66518/#comment281661> It might be efficient to return List<AtlasGlossaryTermHeader>, instead of List<AtlasGlossaryTerm>. webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java Lines 628 (patched) <https://reviews.apache.org/r/66518/#comment281660> It might be efficient to return List<AtlasGlossaryCategoryHeader>, instead of List<AtlasGlossaryCategory> webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java Lines 659 (patched) <https://reviews.apache.org/r/66518/#comment281659> It might be efficient to return List<AtlasGlossaryTermHeader>, instead of List<AtlasGlossaryTerm>. webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java Lines 691 (patched) <https://reviews.apache.org/r/66518/#comment281658> It might be efficient to return List<AtlasGlossaryTermHeader>, instead of List<AtlasGlossaryTerm> webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java Lines 754 (patched) <https://reviews.apache.org/r/66518/#comment281657> Consider replacing List<AtlasEntityHeader> with List<AtlasObjectId>. webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java Lines 780 (patched) <https://reviews.apache.org/r/66518/#comment281656> Consider replacing List<AtlasEntityHeader> with List<AtlasObjectId>. - Madhan Neethiraj On April 9, 2018, 11:13 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66518/ > ----------------------------------------------------------- > > (Updated April 9, 2018, 11:13 p.m.) > > > Review request for atlas, Apoorv Naik, Ashutosh Mestry, keval bhatt, pratik > pandey, and Sarath Subramanian. > > > Bugs: ATLAS-2534 > https://issues.apache.org/jira/browse/ATLAS-2534 > > > Repository: atlas > > > Description > ------- > > (review for patch in ATLAS-2534, by @Apoorv Naik) > > REST API to support Atlas Glossary feature > > > Diffs > ----- > > addons/models/0000-Area0/0011-glossary_model.json PRE-CREATION > > graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphQuery.java > 7bdbeabf0 > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/tinkerpop/query/NativeTinkerpopGraphQuery.java > 75665592e > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/tinkerpop/query/TinkerpopGraphQuery.java > 96b9705fa > > graphdb/common/src/main/java/org/apache/atlas/repository/graphdb/tinkerpop/query/expr/OrderByPredicate.java > PRE-CREATION > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraph.java > fdef80025 > > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/query/NativeJanusGraphQuery.java > d3c976df5 > > graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/query/NativeTitan0GraphQuery.java > 2903ae228 > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 997ac68fb > intg/src/main/java/org/apache/atlas/model/AtlasBaseModelObject.java > 688f6f4d2 > intg/src/main/java/org/apache/atlas/model/annotation/AtlasJSON.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/model/glossary/AtlasGlossary.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/glossary/AtlasGlossaryBaseObject.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/glossary/AtlasGlossaryCategory.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/model/glossary/AtlasGlossaryTerm.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/glossary/enums/AtlasTermAssignmentStatus.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/glossary/enums/AtlasTermRelationshipStatus.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasGlossaryHeader.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasRelatedCategoryHeader.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasRelatedTermHeader.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasTermAssignmentHeader.java > PRE-CREATION > > intg/src/main/java/org/apache/atlas/model/glossary/relations/AtlasTermCategorizationHeader.java > PRE-CREATION > intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java > 0e6f9c75e > intg/src/main/java/org/apache/atlas/model/instance/AtlasEntityHeader.java > 9db920067 > intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java > 576847f6e > intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java > 61168f63f > repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/ogm/AbstractDataTransferObject.java > f1a8bc91d > > repository/src/main/java/org/apache/atlas/repository/ogm/AtlasSavedSearchDTO.java > a1a8f598d > > repository/src/main/java/org/apache/atlas/repository/ogm/AtlasUserProfileDTO.java > bcf2b9d27 > repository/src/main/java/org/apache/atlas/repository/ogm/DTORegistry.java > 818960d96 > repository/src/main/java/org/apache/atlas/repository/ogm/DataAccess.java > c99d2f836 > > repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AbstractGlossaryDTO.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryCategoryDTO.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryDTO.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/ogm/glossary/AtlasGlossaryTermDTO.java > PRE-CREATION > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityChangeNotifier.java > b9945d4fc > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java > 13ee2a6df > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java > 31f6d821f > > repository/src/main/java/org/apache/atlas/repository/userprofile/UserProfileService.java > a428b92ee > repository/src/test/java/org/apache/atlas/TestModules.java c901e891c > repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/glossary/PaginationHelperTest.java > PRE-CREATION > > repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java > d8e916d77 > > repository/src/test/java/org/apache/atlas/repository/userprofile/UserProfileServiceTest.java > 8e19f79e8 > > webapp/src/main/java/org/apache/atlas/notification/EntityNotificationListenerV2.java > 01a95cf80 > webapp/src/main/java/org/apache/atlas/web/rest/GlossaryREST.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/66518/diff/1/ > > > Testing > ------- > > Verified that UTs and ITs pass successfully (except for known failures in > unrelated tests - HiveHookIT, Cassendra) > > > Thanks, > > Madhan Neethiraj > >
