You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by David Radley <da...@uk.ibm.com> on 2017/07/24 17:18:41 UTC
Review Request 61085: ATLAS-1983: Add relationship cardinality
validation
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61085/
-----------------------------------------------------------
Review request for atlas, Madhan Neethiraj and Sarath Subramanian.
Repository: atlas
Description
-------
ATLAS-1983: Add relationship cardinality validation
Diffs
-----
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b24f99f6f9337aa10f40e9f10024fe5a345c3540
intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java c17e875d10169753b76fcdb483e2ca85195104b2
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 49e08a070a803a36253b502666947ab92ffe39d3
Diff: https://reviews.apache.org/r/61085/diff/1/
Testing
-------
ran Junits
Successfully added one relationship, then added a second which resulted in the error.
Thanks,
David Radley
Re: Review Request 61085: ATLAS-1983: Add relationship cardinality
validation
Posted by David Radley <da...@uk.ibm.com>.
> On July 24, 2017, 7:15 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
> > Lines 128 (patched)
> > <https://reviews.apache.org/r/61085/diff/1/?file=1781424#file1781424line134>
> >
> > consider using vertex.getEdges(direction, edgeLabel), instead of iterating through edges to find the one with the expected label.
Hi Madhan,
I can't see how to use this method as I am looking for edges with the relationship prefix - so I am iterating and them checking for a substring of the label. This method would match the complete label. If I have misunderstood, let me know.
David.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61085/#review181253
-----------------------------------------------------------
On July 24, 2017, 5:18 p.m., David Radley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61085/
> -----------------------------------------------------------
>
> (Updated July 24, 2017, 5:18 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Sarath Subramanian.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-1983: Add relationship cardinality validation
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b24f99f6f9337aa10f40e9f10024fe5a345c3540
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java c17e875d10169753b76fcdb483e2ca85195104b2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 49e08a070a803a36253b502666947ab92ffe39d3
>
>
> Diff: https://reviews.apache.org/r/61085/diff/1/
>
>
> Testing
> -------
>
> ran Junits
> Successfully added one relationship, then added a second which resulted in the error.
>
>
> Thanks,
>
> David Radley
>
>
Re: Review Request 61085: ATLAS-1983: Add relationship cardinality
validation
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61085/#review181253
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 128 (patched)
<https://reviews.apache.org/r/61085/#comment256778>
consider using vertex.getEdges(direction, edgeLabel), instead of iterating through edges to find the one with the expected label.
- Madhan Neethiraj
On July 24, 2017, 5:18 p.m., David Radley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61085/
> -----------------------------------------------------------
>
> (Updated July 24, 2017, 5:18 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Sarath Subramanian.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-1983: Add relationship cardinality validation
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b24f99f6f9337aa10f40e9f10024fe5a345c3540
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java c17e875d10169753b76fcdb483e2ca85195104b2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 49e08a070a803a36253b502666947ab92ffe39d3
>
>
> Diff: https://reviews.apache.org/r/61085/diff/1/
>
>
> Testing
> -------
>
> ran Junits
> Successfully added one relationship, then added a second which resulted in the error.
>
>
> Thanks,
>
> David Radley
>
>
Re: Review Request 61085: ATLAS-1983: Add relationship cardinality
validation
Posted by David Radley <da...@apache.org>.
> On Aug. 16, 2017, 5:29 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
> > Lines 128 (patched)
> > <https://reviews.apache.org/r/61085/diff/1/?file=1781424#file1781424line134>
> >
> > David,
> >
> > You are effectively looking for edges with label 'AtlasRelationshipDef.RELATIONSHIP_PREFIX + relationshipType'.
> >
> > So instead of retrieving all edges and looking for label prefix 'AtlasRelationshipDef.RELATIONSHIP_PREFIX', followed by a substring() and equals(relationshipType) - wouldn't it be simpler to retrieve edges with the following call:
> >
> > vertex.getEdges(AtlasEdgeDirection.BOTH, AtlasRelationshipDef.RELATIONSHIP_PREFIX + relationshipType)
> >
> > Madhan
Hi Madhan, I had not thought of that - that is much simpler. Thanks :-) , all the best, David.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61085/#review183043
-----------------------------------------------------------
On July 24, 2017, 5:18 p.m., David Radley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61085/
> -----------------------------------------------------------
>
> (Updated July 24, 2017, 5:18 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Sarath Subramanian.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-1983: Add relationship cardinality validation
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b24f99f6f9337aa10f40e9f10024fe5a345c3540
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java c17e875d10169753b76fcdb483e2ca85195104b2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 49e08a070a803a36253b502666947ab92ffe39d3
>
>
> Diff: https://reviews.apache.org/r/61085/diff/1/
>
>
> Testing
> -------
>
> ran Junits
> Successfully added one relationship, then added a second which resulted in the error.
>
>
> Thanks,
>
> David Radley
>
>
Re: Review Request 61085: ATLAS-1983: Add relationship cardinality
validation
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61085/#review183043
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 128 (patched)
<https://reviews.apache.org/r/61085/#comment259037>
David,
You are effectively looking for edges with label 'AtlasRelationshipDef.RELATIONSHIP_PREFIX + relationshipType'.
So instead of retrieving all edges and looking for label prefix 'AtlasRelationshipDef.RELATIONSHIP_PREFIX', followed by a substring() and equals(relationshipType) - wouldn't it be simpler to retrieve edges with the following call:
vertex.getEdges(AtlasEdgeDirection.BOTH, AtlasRelationshipDef.RELATIONSHIP_PREFIX + relationshipType)
Madhan
- Madhan Neethiraj
On July 24, 2017, 5:18 p.m., David Radley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61085/
> -----------------------------------------------------------
>
> (Updated July 24, 2017, 5:18 p.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Sarath Subramanian.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-1983: Add relationship cardinality validation
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b24f99f6f9337aa10f40e9f10024fe5a345c3540
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java c17e875d10169753b76fcdb483e2ca85195104b2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 49e08a070a803a36253b502666947ab92ffe39d3
>
>
> Diff: https://reviews.apache.org/r/61085/diff/1/
>
>
> Testing
> -------
>
> ran Junits
> Successfully added one relationship, then added a second which resulted in the error.
>
>
> Thanks,
>
> David Radley
>
>
Re: Review Request 61085: ATLAS-1983: Add relationship cardinality
validation
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61085/#review183059
-----------------------------------------------------------
Fix it, then Ship it!
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 123 (patched)
<https://reviews.apache.org/r/61085/#comment259050>
consider moving this method to a utility class - GraphHelper, fetch all relationships given a vertex and relationship name.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 128 (patched)
<https://reviews.apache.org/r/61085/#comment259052>
consider refactoring this method with something like:
public static boolean vertexHasRelationshipWithType(AtlasVertex vertex, String relationshipType) {
Iterator<AtlasEdge> iter = GraphHelper.getAdjacentEdgesByLabel(vertex, AtlasEdgeDirection.BOTH, relationshipType);
return (iter != null) ? iter.hasNext() : false;
}
- Sarath Subramanian
On July 24, 2017, 10:18 a.m., David Radley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61085/
> -----------------------------------------------------------
>
> (Updated July 24, 2017, 10:18 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Sarath Subramanian.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-1983: Add relationship cardinality validation
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b24f99f6f9337aa10f40e9f10024fe5a345c3540
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java c17e875d10169753b76fcdb483e2ca85195104b2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 49e08a070a803a36253b502666947ab92ffe39d3
>
>
> Diff: https://reviews.apache.org/r/61085/diff/1/
>
>
> Testing
> -------
>
> ran Junits
> Successfully added one relationship, then added a second which resulted in the error.
>
>
> Thanks,
>
> David Radley
>
>
Re: Review Request 61085: ATLAS-1983: Add relationship cardinality
validation
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61085/#review183074
-----------------------------------------------------------
Consider refactoring the method to sue something like:
public boolean vertexHasRelationshipWithType(AtlasVertex vertex, String relationshipTypeName) {
if (LOG.isDebugEnabled()) {
LOG.debug("vertexHasRelationshipWithType vertex:" + vertex + ",relationshipType=" + relationshipTypeName);
}
String relationshipEdgeLabel = getRelationshipLabel(getTypeName(vertex), relationshipTypeName);
Iterator<AtlasEdge> iter = graphHelper.getAdjacentEdgesByLabel(vertex, BOTH, relationshipEdgeLabel);
return (iter != null) ? iter.hasNext() : false;
}
private String getRelationshipLabel(String typeName, String relationshipTypeName) {
AtlasRelationshipType relationshipType = typeRegistry.getRelationshipTypeByName(relationshipTypeName);
AtlasRelationshipDef relationshipDef = relationshipType.getRelationshipDef();
AtlasEntityType end1Type = relationshipType.getEnd1Type();
AtlasEntityType end2Type = relationshipType.getEnd2Type();
AtlasAttribute attribute = null;
if (StringUtils.equals(end1Type.getTypeName(), typeName)) {
String attributeName = (relationshipDef.getEndDef1() != null) ? relationshipDef.getEndDef1().getName() : null;
attribute = end1Type.getAttribute(attributeName);
} else if (StringUtils.equals(end2Type.getTypeName(), typeName)) {
String attributeName = (relationshipDef.getEndDef2() != null) ? relationshipDef.getEndDef2().getName() : null;
attribute = end2Type.getAttribute(attributeName);
}
return (attribute != null) ? attribute.getRelationshipEdgeLabel() : null;
}
- Sarath Subramanian
On July 24, 2017, 10:18 a.m., David Radley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61085/
> -----------------------------------------------------------
>
> (Updated July 24, 2017, 10:18 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj and Sarath Subramanian.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-1983: Add relationship cardinality validation
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b24f99f6f9337aa10f40e9f10024fe5a345c3540
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java c17e875d10169753b76fcdb483e2ca85195104b2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 49e08a070a803a36253b502666947ab92ffe39d3
>
>
> Diff: https://reviews.apache.org/r/61085/diff/1/
>
>
> Testing
> -------
>
> ran Junits
> Successfully added one relationship, then added a second which resulted in the error.
>
>
> Thanks,
>
> David Radley
>
>