Re: Review Request 57103: Implementation for classification in V2 API

2017-02-28 Thread Suma Shivaprasad 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57103/
---

(Updated Feb. 28, 2017, 9:05 p.m.)


Review request for atlas.


Changes
---

Fixed RC


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 (updated)
-

  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

Re: Review Request 57103: Implementation for classification in V2 API

2017-02-28 Thread Vimal Sharma 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57103/#review167035
---




repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
 (line 847)


Can result of traitNames be put into a HashMap instead so that we can avoid 
the expensive membership check operation in validateClassificationExists


- Vimal Sharma


On Feb. 28, 2017, 7:01 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, 7:01 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
> 
>

Re: Review Request 57103: Implementation for classification in V2 API

2017-02-27 Thread Madhan Neethiraj 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57103/#review167023
---


Ship it!




Ship It!

- Madhan Neethiraj


On Feb. 28, 2017, 7:01 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, 7:01 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
> 
>

Re: Review Request 57103: Implementation for classification in V2 API

2017-02-27 Thread Suma Shivaprasad 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57103/
---

(Updated Feb. 28, 2017, 7:01 a.m.)


Review request for atlas.


Changes
---

fixed review comments


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 (updated)
-

  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

Re: Review Request 57103: Implementation for classification in V2 API

2017-02-27 Thread Madhan Neethiraj 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57103/#review166989
---


Ship it!




Ship It!

- Madhan Neethiraj


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

Re: Review Request 57103: Implementation for classification in V2 API

2017-02-27 Thread Apoorv Naik 

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


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)


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)


traitInstanceVertex -> classificationInstanceVertex



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
 (line 848)


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)


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)


Same here (for Optional as parameter)



webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java (line 209)


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)


Maybe a better name than clss



webapp/src/main/java/org/apache/atlas/web/rest/EntityREST.java (line 270)


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)


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

Re: Review Request 57103: Implementation for classification in V2 API

2017-02-27 Thread Suma Shivaprasad 

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


Changes
---

Fixed review comments


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 (updated)
-

  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

Re: Review Request 57103: Implementation for classification in V2 API

2017-02-27 Thread Madhan Neethiraj 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57103/#review166962
---




repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java
 (line 52)


Classes referenced by newly added import don't seem to be used. Please 
review and update.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
 (line 128)


consider sending 'context' as parameter to addClassification(), so that 
mapClassification() can use this context (and not create a new context).



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
 (line 192)


"entityType" ==> "classificationType"



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
 (line 798)


Consider moving line #798 to outside this for() loop - @#792



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
 (line 214)


"new AtlasEntitiesWithExtInfo()" - consider sending null, as this is not a 
mandatory parameter in lower level methods that map attribute values.


- Madhan Neethiraj


On Feb. 27, 2017, 8:41 p.m., Suma Shivaprasad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57103/
> ---
> 
> (Updated Feb. 27, 2017, 8:41 p.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/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/rest/EntityREST.java d1bef78 
> 
> Diff: https://reviews.apache.org/r/57103/diff/
> 
> 
> Testing
> ---
> 
> Tested through TestEntityREST
> 
> 
> Thanks,
> 
> Suma Shivaprasad
> 
>