Re: Review Request 61247: [ATLAS-1984]: Use AtlasRelatedObjectId to refer to relationship attributes during entity create/update

2017-08-24 Thread Sarath Subramanian 

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

(Updated Aug. 24, 2017, 5:30 p.m.)


Review request for atlas, David Radley and Madhan Neethiraj.


Changes
---

addressed review comments from Madhan


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
hive_table.db => AtlasObjectId
Change this to use AtlasRelatedObjectId with the following structure:
class AtlasRelatedObjectId
{ String relationshipGuid; String displayText; AtlasStruct 
relationshipAttributes; }


Diffs (updated)
-

  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 4304e745 
  intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 427439ca 
  
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/5/

Changes: https://reviews.apache.org/r/61247/diff/4-5/


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

2017-08-24 Thread Madhan Neethiraj 


> On Aug. 24, 2017, 11:58 p.m., Madhan Neethiraj wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java
> > Lines 307 (patched)
> > 
> >
> > It looks like lines between #307 and #329 can be replaced with the 
> > following. This would be easier to read and perhaps a little more 
> > efficient. Please review.
> > 
> > 
> > for (AtlasAttribute attribute : entityType.getAllAttributes().values()) 
> > {
> > AtlasType attrType = attribute.getAttributeType();
> > StringattrName = attribute.getName();
> > 
> > if (entity.hasRelationshipAttribute(attrName)) {
> > visitAttribute(attrType, 
> > entity.getRelationshipAttribute(attrName));
> > } else if (entity.hasAttribute(attrName)) {
> > visitAttribute(attrType, entity.getAttribute(attrName));
> > }
> > }
> 
> Sarath Subramanian wrote:
> entityType.getAllAttributes() doesn't include relationship attributes 
> (allAttributes is a member of AtlasStructType), also same attribute (e.g. 
> columns) might be in both relationshipAttributes and attributes - In this 
> case relationshipAttribute value takes precendence.

ok. Got it. I missed that one earlier.


- Madhan


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


On Aug. 23, 2017, 5:27 p.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61247/
> ---
> 
> (Updated Aug. 23, 2017, 5:27 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
> 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 4304e745 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 427439ca 
>   
> 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/4/
> 
> 
> 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

2017-08-24 Thread Madhan Neethiraj 

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


Fix it, then Ship it!




Sarath - few minor improvements. Rest of the changes look good. Thanks!


repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java
Lines 307 (patched)


It looks like lines between #307 and #329 can be replaced with the 
following. This would be easier to read and perhaps a little more efficient. 
Please review.

for (AtlasAttribute attribute : entityType.getAllAttributes().values()) {
AtlasType attrType = attribute.getAttributeType();
StringattrName = attribute.getName();

if (entity.hasRelationshipAttribute(attrName)) {
visitAttribute(attrType, entity.getRelationshipAttribute(attrName));
} else if (entity.hasAttribute(attrName)) {
visitAttribute(attrType, entity.getAttribute(attrName));
}
}



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 964 (patched)


'val != null' is not necessary, as the following 'val instanceof' would 
handle this.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 982 (patched)


'val != null' is not necessary, as the following 'val instanceof' would 
handle this.


- Madhan Neethiraj


On Aug. 23, 2017, 5:27 p.m., Sarath Subramanian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61247/
> ---
> 
> (Updated Aug. 23, 2017, 5:27 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
> 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 4304e745 
>   intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 427439ca 
>   
> 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/4/
> 
> 
> 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

2017-08-23 Thread Sarath Subramanian 

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

(Updated Aug. 23, 2017, 10:27 a.m.)


Review request for atlas, David Radley and Madhan Neethiraj.


Changes
---

rebased patch


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
hive_table.db => AtlasObjectId
Change this to use AtlasRelatedObjectId with the following structure:
class AtlasRelatedObjectId
{ String relationshipGuid; String displayText; AtlasStruct 
relationshipAttributes; }


Diffs (updated)
-

  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 4304e745 
  intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 427439ca 
  
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/4/

Changes: https://reviews.apache.org/r/61247/diff/3-4/


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

2017-08-08 Thread Sarath Subramanian 


> On Aug. 7, 2017, 8:10 a.m., David Radley wrote:
> > intg/src/main/java/org/apache/atlas/type/AtlasStructType.java
> > Lines 334 (patched)
> > 
> >
> > 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)
> > 
> >
> > 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)
> > 
> >
> > 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
> 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

2017-08-07 Thread David Radley 

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


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)


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)


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)


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

Review Request 61247: [ATLAS-1984]: Use AtlasRelatedObjectId to refer to relationship attributes during entity create/update

2017-07-30 Thread Sarath Subramanian 

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

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
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/type/AtlasStructType.java 277d0fa2 
  intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 3f395419 
  repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 
c47a89e6 
  
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
 b8fd70e5 


Diff: https://reviews.apache.org/r/61247/diff/1/


Testing
---

Tested using POSTMAN REST client

mvn clean package - In Progress


Thanks,

Sarath Subramanian