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 2018/04/10 06:16:07 UTC
Review Request 66520: [ATLAS-2552]: Add propagated classifications and
blocked propagated classifications in GET releationship REST API
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66520/
-----------------------------------------------------------
Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
Bugs: ATLAS-2552
https://issues.apache.org/jira/browse/ATLAS-2552
Repository: atlas
Description
-------
Surface currently propagated classifications and blocked propagated classifications information in the relationship.
Diffs
-----
common/src/main/java/org/apache/atlas/repository/Constants.java 01e49157
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 997ac68f
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 576847f6
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 28636d86
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 31f6d821
Diff: https://reviews.apache.org/r/66520/diff/1/
Testing
-------
Tested in REST client. GET relationship API yields currently propagated classifications and blocked propagated classifications
Thanks,
Sarath Subramanian
Re: Review Request 66520: [ATLAS-2552]: Add propagated
classifications and
blocked propagated classifications in GET relationship API
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66520/#review200936
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 619 (patched)
<https://reviews.apache.org/r/66520/#comment281861>
getBlockedPropagatedClassificationString() retrieves classification vertex from the given entityGuid and classificationName. Instead of this, consider retrieving List<AtlasVertex> from entityRetriever - for both propagated classifications and blocked classifications; and avoid subsequent queries to retrieve classification vertex.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 463 (patched)
<https://reviews.apache.org/r/66520/#comment281860>
getAllClassificationVertices() - this method returns only classification having propagationEnabled=true. If this is intentional, consider renaming this methid to getPropagationEnabledClassificationVertices()
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 466 (patched)
<https://reviews.apache.org/r/66520/#comment281851>
CLASSIFICATION_EDGE_STATE_PROPERTY_KEY condition is not needed. Please review and remove.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 500 (patched)
<https://reviews.apache.org/r/66520/#comment281858>
Why is this change to return only propagationEnabled classifications from getAllClassifications(entity) method? If this is intentional, consider renaming the method to getPropagationEnabledClssifications().
Also, much of this method body is duplicate of the previous method. Consider calling that method to get list of vertices; and have this method only perform vertex -> classification conversion.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 991 (patched)
<https://reviews.apache.org/r/66520/#comment281857>
when blockedClassificationIds is empty, there is no need to retireve classificationVertices in line #990 above. Please review.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 1024 (patched)
<https://reviews.apache.org/r/66520/#comment281855>
No need to initialize here, as it will be reassigned in line #1030.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 1042 (patched)
<https://reviews.apache.org/r/66520/#comment281856>
Replace #1042 - #1051 with the following call, to avoid code duplication:
List<AtlasVertex> classificationVertices = getClassificationVertices(edge);
- Madhan Neethiraj
On April 11, 2018, 6:07 a.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66520/
> -----------------------------------------------------------
>
> (Updated April 11, 2018, 6:07 a.m.)
>
>
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
>
>
> Bugs: ATLAS-2552
> https://issues.apache.org/jira/browse/ATLAS-2552
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Surface currently propagated classifications and blocked propagated classifications information in the relationship.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 01e49157
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 3805c221
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java c133e394
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 28636d86
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 1d945f7f
>
>
> Diff: https://reviews.apache.org/r/66520/diff/3/
>
>
> Testing
> -------
>
> Tested in REST client. GET relationship API yields currently propagated classifications and blocked propagated classifications
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 66520: [ATLAS-2552]: Add propagated
classifications and
blocked propagated classifications in GET relationship API
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66520/#review200972
-----------------------------------------------------------
Fix it, then Ship it!
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java
Lines 197 (patched)
<https://reviews.apache.org/r/66520/#comment281891>
Instead of casting to List<AtlasClassification>, consider the following:
if (CollectionUtils.isNotEmpty((List) propagatedClassifications)) {
this.propagatedClassifications = new ArrayList<>();
for (Object elem : (List) propagatedClassifications) {
if (elem instanceof AtlasClassification) {
this.propagatedClassifications.add((AtlasClassification) elem);
} else if (elem instanceof Map) {
this.propagatedClassifications.add(new AtlasClassification((Map) elem));
}
}
}
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 939 (patched)
<https://reviews.apache.org/r/66520/#comment281892>
setClassifications() => readClassificationsFromEdge()
- Madhan Neethiraj
On April 12, 2018, 6:07 a.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66520/
> -----------------------------------------------------------
>
> (Updated April 12, 2018, 6:07 a.m.)
>
>
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
>
>
> Bugs: ATLAS-2552
> https://issues.apache.org/jira/browse/ATLAS-2552
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Surface currently propagated classifications and blocked propagated classifications information in the relationship.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 01e49157
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 3805c221
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java c133e394
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 0c63cec3
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 28636d86
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 1d945f7f
>
>
> Diff: https://reviews.apache.org/r/66520/diff/4/
>
>
> Testing
> -------
>
> Tested in REST client. GET relationship API yields currently propagated classifications and blocked propagated classifications
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 66520: [ATLAS-2552]: Add propagated
classifications and
blocked propagated classifications in GET relationship API
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66520/
-----------------------------------------------------------
(Updated April 11, 2018, 11:07 p.m.)
Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
Changes
-------
addressed review comments
Bugs: ATLAS-2552
https://issues.apache.org/jira/browse/ATLAS-2552
Repository: atlas
Description
-------
Surface currently propagated classifications and blocked propagated classifications information in the relationship.
Diffs (updated)
-----
common/src/main/java/org/apache/atlas/repository/Constants.java 01e49157
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 3805c221
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java c133e394
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 0c63cec3
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 28636d86
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 1d945f7f
Diff: https://reviews.apache.org/r/66520/diff/4/
Changes: https://reviews.apache.org/r/66520/diff/3-4/
Testing
-------
Tested in REST client. GET relationship API yields currently propagated classifications and blocked propagated classifications
Thanks,
Sarath Subramanian
Re: Review Request 66520: [ATLAS-2552]: Add propagated
classifications and
blocked propagated classifications in GET relationship API
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66520/
-----------------------------------------------------------
(Updated April 10, 2018, 11:07 p.m.)
Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
Changes
-------
addressed review comments
Bugs: ATLAS-2552
https://issues.apache.org/jira/browse/ATLAS-2552
Repository: atlas
Description
-------
Surface currently propagated classifications and blocked propagated classifications information in the relationship.
Diffs (updated)
-----
common/src/main/java/org/apache/atlas/repository/Constants.java 01e49157
graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Edge.java 1d5d4090
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 3805c221
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java c133e394
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 28636d86
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 1d945f7f
Diff: https://reviews.apache.org/r/66520/diff/2/
Changes: https://reviews.apache.org/r/66520/diff/1-2/
Testing
-------
Tested in REST client. GET relationship API yields currently propagated classifications and blocked propagated classifications
Thanks,
Sarath Subramanian
Re: Review Request 66520: [ATLAS-2552]: Add propagated
classifications and
blocked propagated classifications in GET relationship API
Posted by Sarath Subramanian <sa...@apache.org>.
> On April 10, 2018, 12:29 a.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
> > Lines 969 (patched)
> > <https://reviews.apache.org/r/66520/diff/1/?file=1994716#file1994716line970>
> >
> > Consider ignoring badly formatted entries and process the next element in the array.
refactored this method
- Sarath
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66520/#review200796
-----------------------------------------------------------
On April 10, 2018, 11:07 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66520/
> -----------------------------------------------------------
>
> (Updated April 10, 2018, 11:07 p.m.)
>
>
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
>
>
> Bugs: ATLAS-2552
> https://issues.apache.org/jira/browse/ATLAS-2552
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Surface currently propagated classifications and blocked propagated classifications information in the relationship.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 01e49157
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Edge.java 1d5d4090
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 3805c221
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java c133e394
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 28636d86
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 1d945f7f
>
>
> Diff: https://reviews.apache.org/r/66520/diff/2/
>
>
> Testing
> -------
>
> Tested in REST client. GET relationship API yields currently propagated classifications and blocked propagated classifications
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 66520: [ATLAS-2552]: Add propagated
classifications and
blocked propagated classifications in GET relationship API
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66520/#review200796
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 612 (patched)
<https://reviews.apache.org/r/66520/#comment281690>
Instead of storing [ classificationName + ":" + entityGuid ] in the list, it might be efficient to store the vertex ID of the classification instance. Please review.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 463 (patched)
<https://reviews.apache.org/r/66520/#comment281688>
"else" should be removed, so that BOTH will include #464 as well. Please review.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Lines 969 (patched)
<https://reviews.apache.org/r/66520/#comment281689>
Consider ignoring badly formatted entries and process the next element in the array.
- Madhan Neethiraj
On April 10, 2018, 6:16 a.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66520/
> -----------------------------------------------------------
>
> (Updated April 10, 2018, 6:16 a.m.)
>
>
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, and Madhan Neethiraj.
>
>
> Bugs: ATLAS-2552
> https://issues.apache.org/jira/browse/ATLAS-2552
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Surface currently propagated classifications and blocked propagated classifications information in the relationship.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 01e49157
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 997ac68f
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 576847f6
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 28636d86
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 31f6d821
>
>
> Diff: https://reviews.apache.org/r/66520/diff/1/
>
>
> Testing
> -------
>
> Tested in REST client. GET relationship API yields currently propagated classifications and blocked propagated classifications
>
>
> Thanks,
>
> Sarath Subramanian
>
>