atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Apoorv Naik <naik.apo...@gmail.com>
Subject Re: Review Request 57103: Implementation for classification in V2 API
Date Tue, 28 Feb 2017 02:12:34 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message