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/06/21 19:14:14 UTC
Review Request 60285: [ATLAS-1892]: Implement logic to create
relationship attributes in AtlasEntityType
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60285/
-----------------------------------------------------------
Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Suma Shivaprasad.
Bugs: ATLAS-1892
https://issues.apache.org/jira/browse/ATLAS-1892
Repository: atlas
Description
-------
When a new relationshipDef is created, relation attributes needs to be populated in AtlasEntityType during resolveReference stage.
This JIRA also adds UT, IT and addresses review comments in https://reviews.apache.org/r/59769/
Diffs
-----
authorization/src/main/java/org/apache/atlas/authorize/AtlasResourceTypes.java 925b6b13
authorization/src/main/java/org/apache/atlas/authorize/simple/AtlasAuthorizationUtils.java bb3157a3
authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyParser.java acf73882
distro/src/conf/policy-store.txt 47583c1e
intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java 34e932ed
intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java a29f7fb6
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 296c06c2
intg/src/test/java/org/apache/atlas/type/TestAtlasRelationshipType.java 7a4e9fd5
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 8fe48885
webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java 144080ad
Diff: https://reviews.apache.org/r/60285/diff/1/
Testing
-------
UT added and tested.
IT in progress.
Thanks,
Sarath Subramanian
Re: Review Request 60285: [ATLAS-1892]: Implement logic to create
relationship attributes in AtlasEntityType
Posted by David Radley <da...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60285/#review178673
-----------------------------------------------------------
distro/src/conf/policy-store.txt
Line 9 (original), 9 (patched)
<https://reviews.apache.org/r/60285/#comment252818>
Iwonder what tagsync means for relationships
- I assume we are supporting tags of relational attributes - is this what this enables?
- are we supporting classifications on relationships?
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Line 78 (original), 90 (patched)
<https://reviews.apache.org/r/60285/#comment252821>
I am wondering why we need to validate the relationshipDef in relationship instance logic
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Lines 214 (patched)
<https://reviews.apache.org/r/60285/#comment252822>
bad character
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Lines 215 (patched)
<https://reviews.apache.org/r/60285/#comment252823>
I thought we were not supporintg LIST
- David Radley
On June 21, 2017, 7:14 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60285/
> -----------------------------------------------------------
>
> (Updated June 21, 2017, 7:14 p.m.)
>
>
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Suma Shivaprasad.
>
>
> Bugs: ATLAS-1892
> https://issues.apache.org/jira/browse/ATLAS-1892
>
>
> Repository: atlas
>
>
> Description
> -------
>
> When a new relationshipDef is created, relation attributes needs to be populated in AtlasEntityType during resolveReference stage.
> This JIRA also adds UT, IT and addresses review comments in https://reviews.apache.org/r/59769/
>
>
> Diffs
> -----
>
> authorization/src/main/java/org/apache/atlas/authorize/AtlasResourceTypes.java 925b6b13
> authorization/src/main/java/org/apache/atlas/authorize/simple/AtlasAuthorizationUtils.java bb3157a3
> authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyParser.java acf73882
> distro/src/conf/policy-store.txt 47583c1e
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java e8971a80
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java 34e932ed
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java a29f7fb6
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 38b68bbd
> intg/src/test/java/org/apache/atlas/type/TestAtlasRelationshipType.java 7a4e9fd5
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 8fe48885
> webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java 144080ad
>
>
> Diff: https://reviews.apache.org/r/60285/diff/2/
>
>
> Testing
> -------
>
> UT added and tested.
> IT in progress.
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 60285: [ATLAS-1892]: Implement logic to create
relationship attributes in AtlasEntityType
Posted by David Radley <da...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60285/#review179792
-----------------------------------------------------------
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Line 34 (original), 36 (patched)
<https://reviews.apache.org/r/60285/#comment254640>
1|) created 2 entityDefs and a relationshipDef relating them
2) create 2 entity instances
3) create a relationship between the entity instances
4) I then did a get on one of the entities using its guid and do not see the relationship information. I get :
{
"referredEntities": {},
"entity": {
"typeName": "TestEntBB5",
"attributes": {
"namedd": "Test1"
},
"guid": "0c0c0a42-8793-4dfb-9744-8f0b9fc66195",
"status": "ACTIVE",
"createdBy": "admin",
"updatedBy": "admin",
"createTime": 1499350955631,
"updateTime": 1499350955631,
"version": 0,
"classifications": []
}
}
I was expecting to see the relationship attributes with the attribute names with values of AtlasObjectIds and the relationship attributes somewhere. Am I missing something?
- David Radley
On June 22, 2017, 9:51 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60285/
> -----------------------------------------------------------
>
> (Updated June 22, 2017, 9:51 p.m.)
>
>
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Suma Shivaprasad.
>
>
> Bugs: ATLAS-1892
> https://issues.apache.org/jira/browse/ATLAS-1892
>
>
> Repository: atlas
>
>
> Description
> -------
>
> When a new relationshipDef is created, relation attributes needs to be populated in AtlasEntityType during resolveReference stage.
> This JIRA also adds UT, IT and addresses review comments in https://reviews.apache.org/r/59769/
>
>
> Diffs
> -----
>
> authorization/src/main/java/org/apache/atlas/authorize/AtlasResourceTypes.java 925b6b13
> authorization/src/main/java/org/apache/atlas/authorize/simple/AtlasAuthorizationUtils.java bb3157a3
> authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyParser.java acf73882
> distro/src/conf/policy-store.txt 47583c1e
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java e8971a80
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java 34e932ed
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java a29f7fb6
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 38b68bbd
> intg/src/test/java/org/apache/atlas/type/TestAtlasRelationshipType.java 7a4e9fd5
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 8fe48885
> webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java 144080ad
>
>
> Diff: https://reviews.apache.org/r/60285/diff/3/
>
>
> Testing
> -------
>
> UT added and tested.
> IT in progress.
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 60285: [ATLAS-1892]: Implement logic to create
relationship attributes in AtlasEntityType
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60285/#review178772
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On June 22, 2017, 9:51 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60285/
> -----------------------------------------------------------
>
> (Updated June 22, 2017, 9:51 p.m.)
>
>
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Suma Shivaprasad.
>
>
> Bugs: ATLAS-1892
> https://issues.apache.org/jira/browse/ATLAS-1892
>
>
> Repository: atlas
>
>
> Description
> -------
>
> When a new relationshipDef is created, relation attributes needs to be populated in AtlasEntityType during resolveReference stage.
> This JIRA also adds UT, IT and addresses review comments in https://reviews.apache.org/r/59769/
>
>
> Diffs
> -----
>
> authorization/src/main/java/org/apache/atlas/authorize/AtlasResourceTypes.java 925b6b13
> authorization/src/main/java/org/apache/atlas/authorize/simple/AtlasAuthorizationUtils.java bb3157a3
> authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyParser.java acf73882
> distro/src/conf/policy-store.txt 47583c1e
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java e8971a80
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java 34e932ed
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java a29f7fb6
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 38b68bbd
> intg/src/test/java/org/apache/atlas/type/TestAtlasRelationshipType.java 7a4e9fd5
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 8fe48885
> webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java 144080ad
>
>
> Diff: https://reviews.apache.org/r/60285/diff/3/
>
>
> Testing
> -------
>
> UT added and tested.
> IT in progress.
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 60285: [ATLAS-1892]: Implement logic to create
relationship attributes in AtlasEntityType
Posted by David Radley <da...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60285/#review179764
-----------------------------------------------------------
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
Lines 96 (patched)
<https://reviews.apache.org/r/60285/#comment254598>
updateion => update
the error seems to be called from a validate method which is only called for create, so the error label should not be CRUD. Can this error be issues for updates ?
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Lines 200 (patched)
<https://reviews.apache.org/r/60285/#comment254600>
I had merge issues when I tried to apply this patch. for me this resulted in 2 of these methods and a compile error. I think this method has already been committed.
I also got a merge conflict inthe AtlasErrorCode - on 2 comments that looked the same.
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Lines 221 (patched)
<https://reviews.apache.org/r/60285/#comment254601>
shouldn't there be an else here so we can error in the case where we have already use this attribute name .
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Lines 261 (patched)
<https://reviews.apache.org/r/60285/#comment254599>
the messages should have message numbers.
- David Radley
On June 22, 2017, 9:51 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60285/
> -----------------------------------------------------------
>
> (Updated June 22, 2017, 9:51 p.m.)
>
>
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Suma Shivaprasad.
>
>
> Bugs: ATLAS-1892
> https://issues.apache.org/jira/browse/ATLAS-1892
>
>
> Repository: atlas
>
>
> Description
> -------
>
> When a new relationshipDef is created, relation attributes needs to be populated in AtlasEntityType during resolveReference stage.
> This JIRA also adds UT, IT and addresses review comments in https://reviews.apache.org/r/59769/
>
>
> Diffs
> -----
>
> authorization/src/main/java/org/apache/atlas/authorize/AtlasResourceTypes.java 925b6b13
> authorization/src/main/java/org/apache/atlas/authorize/simple/AtlasAuthorizationUtils.java bb3157a3
> authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyParser.java acf73882
> distro/src/conf/policy-store.txt 47583c1e
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java e8971a80
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java 34e932ed
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java a29f7fb6
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 38b68bbd
> intg/src/test/java/org/apache/atlas/type/TestAtlasRelationshipType.java 7a4e9fd5
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 8fe48885
> webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java 144080ad
>
>
> Diff: https://reviews.apache.org/r/60285/diff/3/
>
>
> Testing
> -------
>
> UT added and tested.
> IT in progress.
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 60285: [ATLAS-1892]: Implement logic to create
relationship attributes in AtlasEntityType
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60285/
-----------------------------------------------------------
(Updated June 22, 2017, 2:51 p.m.)
Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Suma Shivaprasad.
Changes
-------
addressed review comments. Moved logic to add relationship attributes to resolveReferencesPhase2()
Bugs: ATLAS-1892
https://issues.apache.org/jira/browse/ATLAS-1892
Repository: atlas
Description
-------
When a new relationshipDef is created, relation attributes needs to be populated in AtlasEntityType during resolveReference stage.
This JIRA also adds UT, IT and addresses review comments in https://reviews.apache.org/r/59769/
Diffs (updated)
-----
authorization/src/main/java/org/apache/atlas/authorize/AtlasResourceTypes.java 925b6b13
authorization/src/main/java/org/apache/atlas/authorize/simple/AtlasAuthorizationUtils.java bb3157a3
authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyParser.java acf73882
distro/src/conf/policy-store.txt 47583c1e
intg/src/main/java/org/apache/atlas/AtlasErrorCode.java e8971a80
intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java 34e932ed
intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java a29f7fb6
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 38b68bbd
intg/src/test/java/org/apache/atlas/type/TestAtlasRelationshipType.java 7a4e9fd5
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 8fe48885
webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java 144080ad
Diff: https://reviews.apache.org/r/60285/diff/3/
Changes: https://reviews.apache.org/r/60285/diff/2-3/
Testing
-------
UT added and tested.
IT in progress.
Thanks,
Sarath Subramanian
Re: Review Request 60285: [ATLAS-1892]: Implement logic to create
relationship attributes in AtlasEntityType
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60285/#review178633
-----------------------------------------------------------
intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Line 97 (original), 98 (patched)
<https://reviews.apache.org/r/60285/#comment252758>
initialize relationshipAttributes to an empty map here. This will be populated during resolveReferencesPhase2(), from AtlasRelationshipType.resolveReferencesPhase2()
intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 144 (patched)
<https://reviews.apache.org/r/60285/#comment252759>
return relationshipAttributes ==> return Collections.unmodifiableMap(relationshipAttributes)
intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java
Lines 147 (patched)
<https://reviews.apache.org/r/60285/#comment252760>
Given relationshipAttributes will be a modifiable map when addRelationshipAttribute() is called (from AtlasRelationshipType.resolveReferences2()) - this method body can be replaced with:
relationshipAttributes.put(attributeName, attribute)
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Lines 66 (patched)
<https://reviews.apache.org/r/60285/#comment252757>
Resolving referenced entity-types should be moved to resolveReferences2() - as the entity-type may not be complete by now i.e. in resolveReferences()
- like resolving of all attributes, superTypes, etc
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Lines 75 (patched)
<https://reviews.apache.org/r/60285/#comment252761>
Consider moving lines #75 and #84 to line #89 i.e. only after validating the types specified at both ends are entities.
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Lines 197 (patched)
<https://reviews.apache.org/r/60285/#comment252763>
end1Type ==> entityType
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Lines 198 (patched)
<https://reviews.apache.org/r/60285/#comment252762>
"end2TypeName" ==> "attributeTypeName"
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Lines 208 (patched)
<https://reviews.apache.org/r/60285/#comment252764>
relationshipAttrDef may not be necessary. Consider the following:
AtlasAttribute relationshipAttr = end1Type.getAttribute(attrName);
- Madhan Neethiraj
On June 21, 2017, 7:14 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60285/
> -----------------------------------------------------------
>
> (Updated June 21, 2017, 7:14 p.m.)
>
>
> Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, Nixon Rodrigues, and Suma Shivaprasad.
>
>
> Bugs: ATLAS-1892
> https://issues.apache.org/jira/browse/ATLAS-1892
>
>
> Repository: atlas
>
>
> Description
> -------
>
> When a new relationshipDef is created, relation attributes needs to be populated in AtlasEntityType during resolveReference stage.
> This JIRA also adds UT, IT and addresses review comments in https://reviews.apache.org/r/59769/
>
>
> Diffs
> -----
>
> authorization/src/main/java/org/apache/atlas/authorize/AtlasResourceTypes.java 925b6b13
> authorization/src/main/java/org/apache/atlas/authorize/simple/AtlasAuthorizationUtils.java bb3157a3
> authorization/src/main/java/org/apache/atlas/authorize/simple/PolicyParser.java acf73882
> distro/src/conf/policy-store.txt 47583c1e
> intg/src/main/java/org/apache/atlas/AtlasErrorCode.java e8971a80
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java 34e932ed
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java a29f7fb6
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 38b68bbd
> intg/src/test/java/org/apache/atlas/type/TestAtlasRelationshipType.java 7a4e9fd5
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 8fe48885
> webapp/src/main/java/org/apache/atlas/web/rest/RelationshipREST.java 144080ad
>
>
> Diff: https://reviews.apache.org/r/60285/diff/2/
>
>
> Testing
> -------
>
> UT added and tested.
> IT in progress.
>
>
> Thanks,
>
> Sarath Subramanian
>
>