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/08/07 15:10:51 UTC
Re: Review Request 61247: [ATLAS-1984]: Use AtlasRelatedObjectId to
refer to relationship attributes during entity create/update
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61247/#review182278
-----------------------------------------------------------
intg/src/main/java/org/apache/atlas/type/AtlasStructType.java
Lines 334 (patched)
<https://reviews.apache.org/r/61247/#comment258173>
Call null be a valid value for a required attribute.
I think there is a differene between not specifying an attribute (i.e.e the key is not there ) and specifying it with null as a value.
intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java
Lines 319 (patched)
<https://reviews.apache.org/r/61247/#comment258174>
I would have thought that map.get("relationshipAttributes") would either be null or a Map.
If relationshipAttributes is not a map - we should issue an error.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Line 380 (original), 395 (patched)
<https://reviews.apache.org/r/61247/#comment258176>
I am not sure what this is doing - it looks like you are adding relationship attribute for legacy relationships and duplicating them in the inverse edge. Do we need to support relaitonship attributes for legacy relationships? It would be good incentive fror peopel to move to the non legacy relationships.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 1437 (patched)
<https://reviews.apache.org/r/61247/#comment258175>
I suggest this be static and junitted.
This method seems to be removing entity attributes when there was a relationship attributes. This appears to be part of the legacy support; if so shouldn't we be only doing this checking for legacy cases abnd issuing errors otherwise.
- David Radley
On July 31, 2017, 3:36 a.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61247/
> -----------------------------------------------------------
>
> (Updated July 31, 2017, 3:36 a.m.)
>
>
> Review request for atlas, David Radley and Madhan Neethiraj.
>
>
> Bugs: ATLAS-1984
> https://issues.apache.org/jira/browse/ATLAS-1984
>
>
> Repository: atlas
>
>
> Description
> -------
>
> AtlasObjectId is used to refer to entity attributes referring to another entity.
> hive_table.columns => List<AtlasObjectId>
> hive_table.db => AtlasObjectId
> Change this to use AtlasRelatedObjectId with the following structure:
> class AtlasRelatedObjectId
> { String relationshipGuid; String displayText; AtlasStruct relationshipAttributes; }
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 41883713
> intg/src/main/java/org/apache/atlas/model/instance/AtlasStruct.java 80f3a664
> intg/src/main/java/org/apache/atlas/type/AtlasStructType.java 277d0fa2
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 3f395419
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java 0210a118
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java b8fd70e5
>
>
> Diff: https://reviews.apache.org/r/61247/diff/2/
>
>
> Testing
> -------
>
> Tested using POSTMAN REST client
>
> mvn clean package - In Progress
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 61247: [ATLAS-1984]: Use AtlasRelatedObjectId to
refer to relationship attributes during entity create/update
Posted by Sarath Subramanian <sa...@apache.org>.
> On Aug. 7, 2017, 8:10 a.m., David Radley wrote:
> > intg/src/main/java/org/apache/atlas/type/AtlasStructType.java
> > Lines 334 (patched)
> > <https://reviews.apache.org/r/61247/diff/2/?file=1785980#file1785980line334>
> >
> > Call null be a valid value for a required attribute.
> >
> > I think there is a differene between not specifying an attribute (i.e.e the key is not there ) and specifying it with null as a value.
Currently null is not accepted as a valid value for required attribute. If we allow null as valid required attribute values, for e.g. hive_table.db is assigned null, will result in dangling table entity without db reference resulting in inconsistencies in search, ui rendering. This is open to discussion and can be scoped out of this JIRA.
> On Aug. 7, 2017, 8:10 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
> > Line 380 (original), 395 (patched)
> > <https://reviews.apache.org/r/61247/diff/2/?file=1785983#file1785983line396>
> >
> > I am not sure what this is doing - it looks like you are adding relationship attribute for legacy relationships and duplicating them in the inverse edge. Do we need to support relaitonship attributes for legacy relationships? It would be good incentive fror peopel to move to the non legacy relationships.
this section adds relationshipattributes to legacy relationships (hive_table.columns) and its inverse edge (hive_column.table). If we restrict adding relationshipAttributes to legacy edges, how do we support systems with legacy models to use some of the relationship features like tag propagation? we might add propagate direction as relationship attributes and we may need this property to decide on tag propagation direction (even for inverse edge).
> On Aug. 7, 2017, 8:10 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
> > Lines 1437 (patched)
> > <https://reviews.apache.org/r/61247/diff/2/?file=1785983#file1785983line1439>
> >
> > I suggest this be static and junitted.
> >
> > This method seems to be removing entity attributes when there was a relationship attributes. This appears to be part of the legacy support; if so shouldn't we be only doing this checking for legacy cases abnd issuing errors otherwise.
1. Legacy Case : relationship attribute values are specified in 'attributes'
2. New case : relationship attribute values are specified in 'relationshipAttributes'
compactAttributes() removes duplicate attribute from 'attributes' if same attribute is mentioned in both - e.g. 'columns' is mentioned in both attributes and relationshipAttributes - the one in relationshipAttributes takes precedence. I think this method would apply to both legacy and newer cases.
- Sarath
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61247/#review182278
-----------------------------------------------------------
On July 30, 2017, 8:36 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61247/
> -----------------------------------------------------------
>
> (Updated July 30, 2017, 8:36 p.m.)
>
>
> Review request for atlas, David Radley and Madhan Neethiraj.
>
>
> Bugs: ATLAS-1984
> https://issues.apache.org/jira/browse/ATLAS-1984
>
>
> Repository: atlas
>
>
> Description
> -------
>
> AtlasObjectId is used to refer to entity attributes referring to another entity.
> hive_table.columns => List<AtlasObjectId>
> hive_table.db => AtlasObjectId
> Change this to use AtlasRelatedObjectId with the following structure:
> class AtlasRelatedObjectId
> { String relationshipGuid; String displayText; AtlasStruct relationshipAttributes; }
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 41883713
> intg/src/main/java/org/apache/atlas/model/instance/AtlasStruct.java 80f3a664
> intg/src/main/java/org/apache/atlas/type/AtlasStructType.java 277d0fa2
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 3f395419
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java 0210a118
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java b8fd70e5
>
>
> Diff: https://reviews.apache.org/r/61247/diff/2/
>
>
> Testing
> -------
>
> Tested using POSTMAN REST client
>
> mvn clean package - In Progress
>
>
> Thanks,
>
> Sarath Subramanian
>
>