You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Sarath Subramanian <sa...@apache.org> on 2017/09/07 22:16:38 UTC
Review Request 62175: [ATLAS-2119]: Implement update and delete
operations for RelationshipREST
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62175/
-----------------------------------------------------------
Review request for atlas.
Bugs: ATLAS-2119
https://issues.apache.org/jira/browse/ATLAS-2119
Repository: atlas
Description
-------
* Implement update and delete relationship in RelationshipREST
* Add propagateTags information to each relationship instance
Diffs
-----
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java ec6161c0
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 020dd457
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java 227f7cd1
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9b273193
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java b0940f67
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java f6aa7bbf
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 94cc5b93
Diff: https://reviews.apache.org/r/62175/diff/1/
Testing
-------
validated using POSTMAN Rest client.
Integration/Unit Tests in progress
Thanks,
Sarath Subramanian
Re: Review Request 62175: [ATLAS-2119]: Implement update and delete
operations for RelationshipREST
Posted by David Radley <da...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62175/#review186223
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 109 (patched)
<https://reviews.apache.org/r/62175/#comment262701>
I suggest that we should not pass through wording to a message. The "null/empty guid" should be part of the text of a new error message. We then get more unique messages to aid debugging and well as easing globalization if we decide to do that later.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 152 (patched)
<https://reviews.apache.org/r/62175/#comment262702>
samr as previous comment
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 161 (patched)
<https://reviews.apache.org/r/62175/#comment262708>
If this edge is soft deleted, then the deleterelationships ignores the edge. We should throw an error here, if there is an attempt to delete an already deleted relationship.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 245 (patched)
<https://reviews.apache.org/r/62175/#comment262709>
Ideally we should only set the properties in the edge if they are specified in the request i.e. all the ones that relationship.getAttribute(attrName); has a value. Unfortunately it doesn ot look like the code differentiates between null values and unspecified attributes.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 254 (patched)
<https://reviews.apache.org/r/62175/#comment262703>
Asis you do not need the ret variable. I suggest keeping it and adding tracing putting out the ret.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Line 252 (original), 302 (patched)
<https://reviews.apache.org/r/62175/#comment262710>
I am wondering why we are deleting the tracing
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Line 288 (original)
<https://reviews.apache.org/r/62175/#comment262711>
why are we deleting the tracing?
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Line 356 (original), 405 (patched)
<https://reviews.apache.org/r/62175/#comment262704>
Shouldn't this method re-use any soft deleted edge?
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Line 360 (original), 408 (patched)
<https://reviews.apache.org/r/62175/#comment262707>
It does not look like ret can be null here?
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Line 363 (original), 411 (patched)
<https://reviews.apache.org/r/62175/#comment262706>
what about relationship attributes - shouldn't we overwrite them here.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 429 (patched)
<https://reviews.apache.org/r/62175/#comment262705>
This looks strange. You check for one value and set the otehr. If this is what you want to do, please add a comment to show your thinking.
- David Radley
On Sept. 7, 2017, 10:16 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62175/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2017, 10:16 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-2119
> https://issues.apache.org/jira/browse/ATLAS-2119
>
>
> Repository: atlas
>
>
> Description
> -------
>
> * Implement update and delete relationship in RelationshipREST
> * Add propagateTags information to each relationship instance
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java ec6161c0
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 020dd457
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java 227f7cd1
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9b273193
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java b0940f67
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java f6aa7bbf
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 94cc5b93
>
>
> Diff: https://reviews.apache.org/r/62175/diff/1/
>
>
> Testing
> -------
>
> validated using POSTMAN Rest client.
>
> Integration/Unit Tests in progress
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 62175: [ATLAS-2119]: Implement update and delete
operations for RelationshipREST
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62175/
-----------------------------------------------------------
(Updated Jan. 24, 2018, 4:59 p.m.)
Review request for atlas.
Changes
-------
addressed review comments.
Bugs: ATLAS-2119
https://issues.apache.org/jira/browse/ATLAS-2119
Repository: atlas
Description
-------
* Implement update and delete relationship in RelationshipREST
* Add propagateTags information to each relationship instance
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java f8829b57
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 1f459db7
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 4da74b94
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java 37a9661a
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9301cfaa
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java b0940f67
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java e7cd51a4
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java b418fef9
Diff: https://reviews.apache.org/r/62175/diff/7/
Changes: https://reviews.apache.org/r/62175/diff/6-7/
Testing
-------
validated using POSTMAN Rest client.
Integration/Unit Tests in progress
Thanks,
Sarath Subramanian
Re: Review Request 62175: [ATLAS-2119]: Implement update and delete
operations for RelationshipREST
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62175/#review195963
-----------------------------------------------------------
Fix it, then Ship it!
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Line 138 (original), 167 (patched)
<https://reviews.apache.org/r/62175/#comment275424>
Consider using Collections.unmodifiableSet() instead of ImmutableSet.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 257 (patched)
<https://reviews.apache.org/r/62175/#comment275425>
Please review if indexes are created for relationship attributes.
webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java
Line 71 (original)
<https://reviews.apache.org/r/62175/#comment275426>
This file has only white space changes. Please review and revert if there are no other changes.
- Madhan Neethiraj
On Jan. 22, 2018, 11:09 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62175/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2018, 11:09 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-2119
> https://issues.apache.org/jira/browse/ATLAS-2119
>
>
> Repository: atlas
>
>
> Description
> -------
>
> * Implement update and delete relationship in RelationshipREST
> * Add propagateTags information to each relationship instance
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java f8829b57
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 1f459db7
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 4da74b94
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java 6a6ac603
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9301cfaa
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java b0940f67
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java e7cd51a4
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java b418fef9
> webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java 0ee14cb9
>
>
> Diff: https://reviews.apache.org/r/62175/diff/6/
>
>
> Testing
> -------
>
> validated using POSTMAN Rest client.
>
> Integration/Unit Tests in progress
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 62175: [ATLAS-2119]: Implement update and delete
operations for RelationshipREST
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62175/
-----------------------------------------------------------
(Updated Jan. 22, 2018, 3:09 p.m.)
Review request for atlas.
Bugs: ATLAS-2119
https://issues.apache.org/jira/browse/ATLAS-2119
Repository: atlas
Description
-------
* Implement update and delete relationship in RelationshipREST
* Add propagateTags information to each relationship instance
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java f8829b57
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 1f459db7
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 4da74b94
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java 6a6ac603
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9301cfaa
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java b0940f67
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java e7cd51a4
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java b418fef9
webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java 0ee14cb9
Diff: https://reviews.apache.org/r/62175/diff/6/
Changes: https://reviews.apache.org/r/62175/diff/5-6/
Testing
-------
validated using POSTMAN Rest client.
Integration/Unit Tests in progress
Thanks,
Sarath Subramanian
Re: Review Request 62175: [ATLAS-2119]: Implement update and delete
operations for RelationshipREST
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62175/
-----------------------------------------------------------
(Updated Oct. 17, 2017, 11:36 a.m.)
Review request for atlas.
Changes
-------
addressed review comments.
Bugs: ATLAS-2119
https://issues.apache.org/jira/browse/ATLAS-2119
Repository: atlas
Description
-------
* Implement update and delete relationship in RelationshipREST
* Add propagateTags information to each relationship instance
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java fb741f8f
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java ec6161c0
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 639077dd
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java 1eb41832
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9b273193
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java b0940f67
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 36cd980f
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 94cc5b93
Diff: https://reviews.apache.org/r/62175/diff/5/
Changes: https://reviews.apache.org/r/62175/diff/4-5/
Testing
-------
validated using POSTMAN Rest client.
Integration/Unit Tests in progress
Thanks,
Sarath Subramanian
Re: Review Request 62175: [ATLAS-2119]: Implement update and delete
operations for RelationshipREST
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62175/#review188042
-----------------------------------------------------------
notification/pom.xml
Line 1 (original), 1 (patched)
<https://reviews.apache.org/r/62175/#comment265119>
This file has only whitespace changes; please revert.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 251 (patched)
<https://reviews.apache.org/r/62175/#comment265118>
tagPropagation is not set in createRelationship() above. Please review.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 266 (patched)
<https://reviews.apache.org/r/62175/#comment265120>
Update of relationships should send out notifications for changes in propagated tags - if any.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 431 (patched)
<https://reviews.apache.org/r/62175/#comment265121>
Why not use tagPropagation setting from the relationShipType?
- Madhan Neethiraj
On Sept. 7, 2017, 10:16 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62175/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2017, 10:16 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-2119
> https://issues.apache.org/jira/browse/ATLAS-2119
>
>
> Repository: atlas
>
>
> Description
> -------
>
> * Implement update and delete relationship in RelationshipREST
> * Add propagateTags information to each relationship instance
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java bf098062
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java ec6161c0
> notification/pom.xml 9b369403
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 639077dd
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java 1eb41832
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9b273193
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java b0940f67
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 36cd980f
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 94cc5b93
>
>
> Diff: https://reviews.apache.org/r/62175/diff/4/
>
>
> Testing
> -------
>
> validated using POSTMAN Rest client.
>
> Integration/Unit Tests in progress
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 62175: [ATLAS-2119]: Implement update and delete
operations for RelationshipREST
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62175/#review188043
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Line 139 (original), 172 (patched)
<https://reviews.apache.org/r/62175/#comment265117>
Delete of relationships should send out notifications for changes in propagated tags - if any.
- Madhan Neethiraj
On Sept. 7, 2017, 10:16 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62175/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2017, 10:16 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-2119
> https://issues.apache.org/jira/browse/ATLAS-2119
>
>
> Repository: atlas
>
>
> Description
> -------
>
> * Implement update and delete relationship in RelationshipREST
> * Add propagateTags information to each relationship instance
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java bf098062
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java ec6161c0
> notification/pom.xml 9b369403
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 639077dd
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java 1eb41832
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9b273193
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java b0940f67
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 36cd980f
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 94cc5b93
>
>
> Diff: https://reviews.apache.org/r/62175/diff/4/
>
>
> Testing
> -------
>
> validated using POSTMAN Rest client.
>
> Integration/Unit Tests in progress
>
>
> Thanks,
>
> Sarath Subramanian
>
>