----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57103/#review166987 -----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java (line 466) <https://reviews.apache.org/r/57103/#comment239082> Arrays.asList or Collection.singletonList would be better in terms of readability repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java (line 197) <https://reviews.apache.org/r/57103/#comment239083> Can we use AtlasGraphUtilsV1.setProperty(ret, Constants.SUPER_TYPES_PROPERTY_KEY, entityType.getAllSuperTypes()); (if it does the same thing)? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java (line 801) <https://reviews.apache.org/r/57103/#comment239084> traitInstanceVertex -> classificationInstanceVertex repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java (line 848) <https://reviews.apache.org/r/57103/#comment239085> Would it be better to perform the exists check before processing so that we can avoid graph operations and rollbacks ? repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java (line 224) <https://reviews.apache.org/r/57103/#comment239086> I think this might be misuse of Optional, usually only return types need to be optional so that the return values can be evaluated quickly and bail out if needed. repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java (line 276) <https://reviews.apache.org/r/57103/#comment239087> Same here (for Optional as parameter) webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java (line 209) <https://reviews.apache.org/r/57103/#comment239088> JavaDoc says classifications but I think this will return a specific one. Please check. webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java (line 238) <https://reviews.apache.org/r/57103/#comment239089> Maybe a better name than clss webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java (line 270) <https://reviews.apache.org/r/57103/#comment239090> Shouldn't be consuming anything here, usually DELETEs don't accept any body. webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java (line 348) <https://reviews.apache.org/r/57103/#comment239091> "empty entity list" -> "empty guid list" - Apoorv Naik On Feb. 28, 2017, 1:58 a.m., Suma Shivaprasad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57103/ > ----------------------------------------------------------- > > (Updated Feb. 28, 2017, 1:58 a.m.) > > > Review request for atlas. > > > Bugs: ATLAS-1601 > https://issues.apache.org/jira/browse/ATLAS-1601 > > > Repository: atlas > > > Description > ------- > > Add support for classification store, REST API for addition, deletion and gets > > > Diffs > ----- > > intg/src/main/java/org/apache/atlas/AtlasErrorCode.java d58c514 > > repository/src/main/java/org/apache/atlas/repository/converters/AtlasInstanceConverter.java > 621b32f > > repository/src/main/java/org/apache/atlas/repository/store/graph/AtlasEntityStore.java > 6c372b3 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java > c84f169 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java > 09f69db > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java > 3ba2190 > > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityMutationContext.java > 23e825e > webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java > d7adb3a > webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java d1bef78 > > Diff: https://reviews.apache.org/r/57103/diff/ > > > Testing > ------- > > Tested through TestEntityREST > > > Thanks, > > Suma Shivaprasad > >
