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/07/18 06:25:48 UTC
Review Request 60938: [ATLAS-1959]: Enhance relationship attributes to
support different cardinality mappings
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60938/
-----------------------------------------------------------
Review request for atlas and Madhan Neethiraj.
Bugs: ATLAS-1959
https://issues.apache.org/jira/browse/ATLAS-1959
Repository: atlas
Description
-------
* Improve relationship model to support create/update operations and the following cardinalities (previously supported using inverseReference):
1 to 1
1 to many
many to 1
many to many
* Change legacyLabel flag in AtlasRelationshipEndDef to boolean flag.
* Add unit tests for the above cases.
Diffs
-----
addons/models/0010-base_model.json 303f3796
addons/models/0030-hive_model.json a795f0f3
addons/models/0050-falcon_model.json 7755fa86
addons/models/0060-hbase_model.json 1d264df4
addons/models/0080-storm_model.json 25360ff0
intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 68da6af1
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 2de9bdf0
intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java fc820d49
intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java f80ea895
intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java e94dd190
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 841b66f7
intg/src/main/java/org/apache/atlas/type/AtlasStructType.java f97d7674
intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java PRE-CREATION
intg/src/test/java/org/apache/atlas/TestUtilsV2.java 9774583d
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 6f6d74bc
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java 12e8bb1f
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 3ff6fbef
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java d4fdc257
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java 157f8cd2
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java f4257be7
repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java de8e7ef3
repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java d9017319
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreHardDeleteV1Test.java PRE-CREATION
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreSoftDeleteV1Test.java PRE-CREATION
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 67702231
repository/src/test/resources/logging-v1-full/2c8c2622-c7aa-43be-8956-f47dd5f6bac4.json PRE-CREATION
repository/src/test/resources/logging-v1-full/63a4b6f5-d0a4-484c-8cab-61c0c14b6c69.json PRE-CREATION
repository/src/test/resources/logging-v1-full/974cbc3a-c9c9-4776-ae0e-357df2ea055f.json PRE-CREATION
repository/src/test/resources/logging-v1-full/atlas-export-info.json PRE-CREATION
repository/src/test/resources/logging-v1-full/atlas-export-order.json PRE-CREATION
repository/src/test/resources/logging-v1-full/atlas-typesdef.json PRE-CREATION
repository/src/test/resources/logging-v1-full/ec2ed3d2-b603-4948-bf2d-dde49411000a.json PRE-CREATION
repository/src/test/resources/sales-v1-full/221de408-7246-4988-82f7-fe889dad032e.json PRE-CREATION
repository/src/test/resources/sales-v1-full/42f82e22-ebb9-4f2e-b93a-6d436b239fad.json PRE-CREATION
repository/src/test/resources/sales-v1-full/7178713e-fc15-4406-86dd-da5a4f077257.json PRE-CREATION
repository/src/test/resources/sales-v1-full/84d079fc-6177-4937-a232-944aa35e568d.json PRE-CREATION
repository/src/test/resources/sales-v1-full/aa0bb6cf-f3db-4fb7-9c69-c67447b3b6ad.json PRE-CREATION
repository/src/test/resources/sales-v1-full/atlas-export-info.json PRE-CREATION
repository/src/test/resources/sales-v1-full/atlas-export-order.json PRE-CREATION
repository/src/test/resources/sales-v1-full/atlas-typesdef.json PRE-CREATION
repository/src/test/resources/sales-v1-full/bfe88eb8-7556-403c-8210-647013f44a44.json PRE-CREATION
repository/src/test/resources/sales-v1-full/ee35e6b3-8d6f-4815-9acd-3ef24c78ac49.json PRE-CREATION
repository/src/test/resources/sales-v1-full/ef3354e4-38c5-48fa-879f-552b1942034e.json PRE-CREATION
repository/src/test/resources/sales-v1-full/efdfafa5-d74b-4733-8f36-fdb05bc4adcf.json PRE-CREATION
repository/src/test/resources/sales-v1-full/f2dd85a6-3bc6-414e-b6bb-ca385592da28.json PRE-CREATION
repository/src/test/resources/sales-v1-full/f69046b1-7518-45e6-a86b-5d34007251a5.json PRE-CREATION
repository/src/test/resources/sales-v1-full/fe91bf93-eb0c-4638-8361-15937390c810.json PRE-CREATION
Diff: https://reviews.apache.org/r/60938/diff/1/
Testing
-------
added unit test - AtlasRelationshipStoreV1Test
mvn clean package - succeeded with no errors.
Thanks,
Sarath Subramanian
Re: Review Request 60938: [ATLAS-1959]: Enhance relationship
attributes to support different cardinality mappings
Posted by David Radley <da...@uk.ibm.com>.
> On July 19, 2017, 3:11 p.m., David Radley wrote:
> > addons/models/0080-storm_model.json
> > Line 150 (original), 150 (patched)
> > <https://reviews.apache.org/r/60938/diff/2/?file=1778156#file1778156line150>
> >
> > shouldn't this be aggregation ?
>
> Sarath Subramanian wrote:
> since a storm node can be shared acoss multiple storm topologies and not tied to a single topology made it as ASSOCIATION
It seems better to have this as aggregation to should some element of containment. From your description , the topology contains the storm node. The storm node could also be contained in otehr topologies . If htis is the case - it should be AGGREGATION I think.
> On July 19, 2017, 3:11 p.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
> > Lines 256 (patched)
> > <https://reviews.apache.org/r/60938/diff/2/?file=1778170#file1778170line257>
> >
> > Does the API still allow creation of entities with constaints - how will this come through to thie code.
>
> Sarath Subramanian wrote:
> yes, to support backward compatibility entity creation with constraints is still allowed.
Thanks Sarath for the explanation. I think I get it. I wonder if there is a cleaner way we can design this.
What I do not like about this design is that the user could specify RelationshipDefs that do not quite match the existing constraint definitions. Also I am not keen on an flag with legacy in - as todays new functions is tomorrow legacy. the models also look messy with all the duplication.
I assume for the 'legacy case', the requirement is to be able to specify propagate tags- I assume there is no compelling requirement to add relationshipDef attributes to the old API form.
If this is the case, I suggest we remove the legacy flag from RelationshipDef and remove the duplicate RelationshipDefs from the models.
Instead we should add a flag to the constraint API form as an indicator to propagate tags. This would be so much cleaner - and would not need to expose anything legacy in the RelationshipDef API and would mean that the user could not mismatch relationshipDefs with existing entityDef attributeDef constraint definitions. The model files would also not have all the duplication that they currently have.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60938/#review180926
-----------------------------------------------------------
On July 19, 2017, 7:27 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60938/
> -----------------------------------------------------------
>
> (Updated July 19, 2017, 7:27 p.m.)
>
>
> Review request for atlas and Madhan Neethiraj.
>
>
> Bugs: ATLAS-1959
> https://issues.apache.org/jira/browse/ATLAS-1959
>
>
> Repository: atlas
>
>
> Description
> -------
>
> * Improve relationship model to support create/update operations and the following cardinalities (previously supported using inverseReference):
> 1 to 1
> 1 to many
> many to 1
> many to many
>
>
> * Change legacyLabel flag in AtlasRelationshipEndDef to boolean flag.
> * Add unit tests for the above cases.
>
>
> Diffs
> -----
>
> addons/models/0010-base_model.json 303f3796
> addons/models/0030-hive_model.json a795f0f3
> addons/models/0050-falcon_model.json 7755fa86
> addons/models/0060-hbase_model.json 1d264df4
> addons/models/0080-storm_model.json 25360ff0
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 68da6af1
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 2de9bdf0
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java fc820d49
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java f80ea895
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java e94dd190
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 841b66f7
> intg/src/main/java/org/apache/atlas/type/AtlasStructType.java f97d7674
> intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java PRE-CREATION
> intg/src/test/java/org/apache/atlas/TestUtilsV2.java 9774583d
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 6f6d74bc
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java 12e8bb1f
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 3ff6fbef
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java d4fdc257
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java 157f8cd2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java f4257be7
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java 404225c2
> repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java d9017319
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreHardDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreSoftDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 67702231
>
>
> Diff: https://reviews.apache.org/r/60938/diff/4/
>
>
> Testing
> -------
>
> added unit test - AtlasRelationshipStoreV1Test
>
> mvn clean package - succeeded with no errors.
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 60938: [ATLAS-1959]: Enhance relationship
attributes to support different cardinality mappings
Posted by Sarath Subramanian <sa...@apache.org>.
> On July 19, 2017, 8:11 a.m., David Radley wrote:
> > addons/models/0010-base_model.json
> > Line 105 (original), 105 (patched)
> > <https://reviews.apache.org/r/60938/diff/2/?file=1778152#file1778152line105>
> >
> > I am not sunderstanding why some of these endpoints have the islegacyattribute on one end and some have it on both. It seems to be somehting to do with whetehr the relationships is composition or aggregation ; in that case the container end has the legacy flag.
if a regular attribute is also defined as a relationship attribute then the attribute endpoint is tagged with legacy flag.
for e.g. hive_table.columns and hive_column.table is defined as regular attribute using inverseRef/ownedRef constraint and as a relationship attribute in relationshipDef) - legacy flag on both ends
In "hive_db_tables" relationshipDef, hive_table.db is a regular and relationship attribute (tagged as legacy) but hive_db.tables is only relationship attribute (not tagged as legacy). Hope this helps.
> On July 19, 2017, 8:11 a.m., David Radley wrote:
> > addons/models/0030-hive_model.json
> > Line 554 (original), 554 (patched)
> > <https://reviews.apache.org/r/60938/diff/2/?file=1778153#file1778153line554>
> >
> > why has this composition got the legacy flag on both ends ? Whereas otehr only have it one one .
commented above
> On July 19, 2017, 8:11 a.m., David Radley wrote:
> > addons/models/0080-storm_model.json
> > Line 150 (original), 150 (patched)
> > <https://reviews.apache.org/r/60938/diff/2/?file=1778156#file1778156line150>
> >
> > shouldn't this be aggregation ?
since a storm node can be shared acoss multiple storm topologies and not tied to a single topology made it as ASSOCIATION
> On July 19, 2017, 8:11 a.m., David Radley wrote:
> > addons/models/0080-storm_model.json
> > Line 158 (original), 158 (patched)
> > <https://reviews.apache.org/r/60938/diff/2/?file=1778156#file1778156line158>
> >
> > do we need the isLagacyAttribute here?
not needed, since "topolgies" attribute is a new relationship attribute and not a regular attribute
> On July 19, 2017, 8:11 a.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
> > Lines 256 (patched)
> > <https://reviews.apache.org/r/60938/diff/2/?file=1778170#file1778170line257>
> >
> > Does the API still allow creation of entities with constaints - how will this come through to thie code.
yes, to support backward compatibility entity creation with constraints is still allowed.
- Sarath
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60938/#review180926
-----------------------------------------------------------
On July 17, 2017, 11:25 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60938/
> -----------------------------------------------------------
>
> (Updated July 17, 2017, 11:25 p.m.)
>
>
> Review request for atlas and Madhan Neethiraj.
>
>
> Bugs: ATLAS-1959
> https://issues.apache.org/jira/browse/ATLAS-1959
>
>
> Repository: atlas
>
>
> Description
> -------
>
> * Improve relationship model to support create/update operations and the following cardinalities (previously supported using inverseReference):
> 1 to 1
> 1 to many
> many to 1
> many to many
>
>
> * Change legacyLabel flag in AtlasRelationshipEndDef to boolean flag.
> * Add unit tests for the above cases.
>
>
> Diffs
> -----
>
> addons/models/0010-base_model.json 303f3796
> addons/models/0030-hive_model.json a795f0f3
> addons/models/0050-falcon_model.json 7755fa86
> addons/models/0060-hbase_model.json 1d264df4
> addons/models/0080-storm_model.json 25360ff0
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 68da6af1
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 2de9bdf0
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java fc820d49
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java f80ea895
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java e94dd190
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 841b66f7
> intg/src/main/java/org/apache/atlas/type/AtlasStructType.java f97d7674
> intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java PRE-CREATION
> intg/src/test/java/org/apache/atlas/TestUtilsV2.java 9774583d
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 6f6d74bc
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java 12e8bb1f
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 3ff6fbef
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java d4fdc257
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java 157f8cd2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java f4257be7
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java de8e7ef3
> repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java d9017319
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreHardDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreSoftDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 67702231
>
>
> Diff: https://reviews.apache.org/r/60938/diff/3/
>
>
> Testing
> -------
>
> added unit test - AtlasRelationshipStoreV1Test
>
> mvn clean package - succeeded with no errors.
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 60938: [ATLAS-1959]: Enhance relationship
attributes to support different cardinality mappings
Posted by Sarath Subramanian <sa...@apache.org>.
> On July 19, 2017, 8:11 a.m., David Radley wrote:
> > addons/models/0080-storm_model.json
> > Line 150 (original), 150 (patched)
> > <https://reviews.apache.org/r/60938/diff/2/?file=1778156#file1778156line150>
> >
> > shouldn't this be aggregation ?
>
> Sarath Subramanian wrote:
> since a storm node can be shared acoss multiple storm topologies and not tied to a single topology made it as ASSOCIATION
>
> David Radley wrote:
> It seems better to have this as aggregation to should some element of containment. From your description , the topology contains the storm node. The storm node could also be contained in otehr topologies . If htis is the case - it should be AGGREGATION I think.
will change to AGGREGATION
- Sarath
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60938/#review180926
-----------------------------------------------------------
On July 19, 2017, 12:27 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60938/
> -----------------------------------------------------------
>
> (Updated July 19, 2017, 12:27 p.m.)
>
>
> Review request for atlas and Madhan Neethiraj.
>
>
> Bugs: ATLAS-1959
> https://issues.apache.org/jira/browse/ATLAS-1959
>
>
> Repository: atlas
>
>
> Description
> -------
>
> * Improve relationship model to support create/update operations and the following cardinalities (previously supported using inverseReference):
> 1 to 1
> 1 to many
> many to 1
> many to many
>
>
> * Change legacyLabel flag in AtlasRelationshipEndDef to boolean flag.
> * Add unit tests for the above cases.
>
>
> Diffs
> -----
>
> addons/models/0010-base_model.json 303f3796
> addons/models/0030-hive_model.json a795f0f3
> addons/models/0050-falcon_model.json 7755fa86
> addons/models/0060-hbase_model.json 1d264df4
> addons/models/0080-storm_model.json 25360ff0
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 68da6af1
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 2de9bdf0
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java fc820d49
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java f80ea895
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java e94dd190
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 841b66f7
> intg/src/main/java/org/apache/atlas/type/AtlasStructType.java f97d7674
> intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java PRE-CREATION
> intg/src/test/java/org/apache/atlas/TestUtilsV2.java 9774583d
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 6f6d74bc
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java 12e8bb1f
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 3ff6fbef
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java d4fdc257
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java 157f8cd2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java f4257be7
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java 404225c2
> repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java d9017319
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreHardDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreSoftDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 67702231
>
>
> Diff: https://reviews.apache.org/r/60938/diff/4/
>
>
> Testing
> -------
>
> added unit test - AtlasRelationshipStoreV1Test
>
> mvn clean package - succeeded with no errors.
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 60938: [ATLAS-1959]: Enhance relationship
attributes to support different cardinality mappings
Posted by David Radley <da...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60938/#review180926
-----------------------------------------------------------
addons/models/0010-base_model.json
Line 105 (original), 105 (patched)
<https://reviews.apache.org/r/60938/#comment256275>
I am not sunderstanding why some of these endpoints have the islegacyattribute on one end and some have it on both. It seems to be somehting to do with whetehr the relationships is composition or aggregation ; in that case the container end has the legacy flag.
addons/models/0030-hive_model.json
Line 554 (original), 554 (patched)
<https://reviews.apache.org/r/60938/#comment256278>
why has this composition got the legacy flag on both ends ? Whereas otehr only have it one one .
addons/models/0080-storm_model.json
Line 150 (original), 150 (patched)
<https://reviews.apache.org/r/60938/#comment256276>
shouldn't this be aggregation ?
addons/models/0080-storm_model.json
Line 158 (original), 158 (patched)
<https://reviews.apache.org/r/60938/#comment256273>
do we need the isLagacyAttribute here?
intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java
Line 64 (original), 63 (patched)
<https://reviews.apache.org/r/60938/#comment256267>
is is
also I think we should document what we mean by a legacy attribute and how it behaves in different relationshionships. Also we should recommend here in the API docs when users should or should not set this flag and why.
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Line 99 (original), 103 (patched)
<https://reviews.apache.org/r/60938/#comment256270>
I am not sure what this comment means. If we have the flag on both ends, we coudl generate the label using eitehr entity; there does not seem to be any code for this case.
I suggest these cases need junits
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java
Line 230 (original), 259 (patched)
<https://reviews.apache.org/r/60938/#comment256271>
same - I am not sure what this means
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
Lines 256 (patched)
<https://reviews.apache.org/r/60938/#comment256272>
Does the API still allow creation of entities with constaints - how will this come through to thie code.
- David Radley
On July 18, 2017, 6:25 a.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60938/
> -----------------------------------------------------------
>
> (Updated July 18, 2017, 6:25 a.m.)
>
>
> Review request for atlas and Madhan Neethiraj.
>
>
> Bugs: ATLAS-1959
> https://issues.apache.org/jira/browse/ATLAS-1959
>
>
> Repository: atlas
>
>
> Description
> -------
>
> * Improve relationship model to support create/update operations and the following cardinalities (previously supported using inverseReference):
> 1 to 1
> 1 to many
> many to 1
> many to many
>
>
> * Change legacyLabel flag in AtlasRelationshipEndDef to boolean flag.
> * Add unit tests for the above cases.
>
>
> Diffs
> -----
>
> addons/models/0010-base_model.json 303f3796
> addons/models/0030-hive_model.json a795f0f3
> addons/models/0050-falcon_model.json 7755fa86
> addons/models/0060-hbase_model.json 1d264df4
> addons/models/0080-storm_model.json 25360ff0
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 68da6af1
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 2de9bdf0
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java fc820d49
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java f80ea895
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java e94dd190
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 841b66f7
> intg/src/main/java/org/apache/atlas/type/AtlasStructType.java f97d7674
> intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java PRE-CREATION
> intg/src/test/java/org/apache/atlas/TestUtilsV2.java 9774583d
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 6f6d74bc
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java 12e8bb1f
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 3ff6fbef
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java d4fdc257
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java 157f8cd2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java f4257be7
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java de8e7ef3
> repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java d9017319
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreHardDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreSoftDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 67702231
>
>
> Diff: https://reviews.apache.org/r/60938/diff/2/
>
>
> Testing
> -------
>
> added unit test - AtlasRelationshipStoreV1Test
>
> mvn clean package - succeeded with no errors.
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 60938: [ATLAS-1959]: Enhance relationship
attributes to support different cardinality mappings
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60938/#review181009
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On July 19, 2017, 7:27 p.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60938/
> -----------------------------------------------------------
>
> (Updated July 19, 2017, 7:27 p.m.)
>
>
> Review request for atlas and Madhan Neethiraj.
>
>
> Bugs: ATLAS-1959
> https://issues.apache.org/jira/browse/ATLAS-1959
>
>
> Repository: atlas
>
>
> Description
> -------
>
> * Improve relationship model to support create/update operations and the following cardinalities (previously supported using inverseReference):
> 1 to 1
> 1 to many
> many to 1
> many to many
>
>
> * Change legacyLabel flag in AtlasRelationshipEndDef to boolean flag.
> * Add unit tests for the above cases.
>
>
> Diffs
> -----
>
> addons/models/0010-base_model.json 303f3796
> addons/models/0030-hive_model.json a795f0f3
> addons/models/0050-falcon_model.json 7755fa86
> addons/models/0060-hbase_model.json 1d264df4
> addons/models/0080-storm_model.json 25360ff0
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 68da6af1
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 2de9bdf0
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java fc820d49
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java f80ea895
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java e94dd190
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 841b66f7
> intg/src/main/java/org/apache/atlas/type/AtlasStructType.java f97d7674
> intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java PRE-CREATION
> intg/src/test/java/org/apache/atlas/TestUtilsV2.java 9774583d
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 6f6d74bc
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java 12e8bb1f
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 3ff6fbef
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java d4fdc257
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java 157f8cd2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java f4257be7
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java 404225c2
> repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java d9017319
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreHardDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreSoftDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 67702231
>
>
> Diff: https://reviews.apache.org/r/60938/diff/4/
>
>
> Testing
> -------
>
> added unit test - AtlasRelationshipStoreV1Test
>
> mvn clean package - succeeded with no errors.
>
>
> Thanks,
>
> Sarath Subramanian
>
>
Re: Review Request 60938: [ATLAS-1959]: Enhance relationship
attributes to support different cardinality mappings
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60938/
-----------------------------------------------------------
(Updated July 19, 2017, 12:27 p.m.)
Review request for atlas and Madhan Neethiraj.
Changes
-------
addressed review comments
Bugs: ATLAS-1959
https://issues.apache.org/jira/browse/ATLAS-1959
Repository: atlas
Description
-------
* Improve relationship model to support create/update operations and the following cardinalities (previously supported using inverseReference):
1 to 1
1 to many
many to 1
many to many
* Change legacyLabel flag in AtlasRelationshipEndDef to boolean flag.
* Add unit tests for the above cases.
Diffs (updated)
-----
addons/models/0010-base_model.json 303f3796
addons/models/0030-hive_model.json a795f0f3
addons/models/0050-falcon_model.json 7755fa86
addons/models/0060-hbase_model.json 1d264df4
addons/models/0080-storm_model.json 25360ff0
intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 68da6af1
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 2de9bdf0
intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java fc820d49
intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java f80ea895
intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java e94dd190
intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 841b66f7
intg/src/main/java/org/apache/atlas/type/AtlasStructType.java f97d7674
intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java PRE-CREATION
intg/src/test/java/org/apache/atlas/TestUtilsV2.java 9774583d
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 6f6d74bc
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java 12e8bb1f
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 3ff6fbef
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java d4fdc257
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java 157f8cd2
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java f4257be7
repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java 404225c2
repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java d9017319
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreHardDeleteV1Test.java PRE-CREATION
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreSoftDeleteV1Test.java PRE-CREATION
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 67702231
Diff: https://reviews.apache.org/r/60938/diff/4/
Changes: https://reviews.apache.org/r/60938/diff/3-4/
Testing
-------
added unit test - AtlasRelationshipStoreV1Test
mvn clean package - succeeded with no errors.
Thanks,
Sarath Subramanian
Re: Review Request 60938: [ATLAS-1959]: Enhance relationship
attributes to support different cardinality mappings
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60938/#review180899
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java
Line 466 (original), 468 (patched)
<https://reviews.apache.org/r/60938/#comment256223>
for relationshipAttributes, it is not neecessary to retrieve complete entity (and store it in entityExtInfo.referredEntities) - it could be expensive to retrieve and transmit when large number of entities are referenced (for example hive_db.tables referring to 1000s of tables). Storing AtlasObjectId (or List<AtlasObjectId>) is enough.
Same comment for line #474 below.
- Madhan Neethiraj
On July 18, 2017, 6:25 a.m., Sarath Subramanian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60938/
> -----------------------------------------------------------
>
> (Updated July 18, 2017, 6:25 a.m.)
>
>
> Review request for atlas and Madhan Neethiraj.
>
>
> Bugs: ATLAS-1959
> https://issues.apache.org/jira/browse/ATLAS-1959
>
>
> Repository: atlas
>
>
> Description
> -------
>
> * Improve relationship model to support create/update operations and the following cardinalities (previously supported using inverseReference):
> 1 to 1
> 1 to many
> many to 1
> many to many
>
>
> * Change legacyLabel flag in AtlasRelationshipEndDef to boolean flag.
> * Add unit tests for the above cases.
>
>
> Diffs
> -----
>
> addons/models/0010-base_model.json 303f3796
> addons/models/0030-hive_model.json a795f0f3
> addons/models/0050-falcon_model.json 7755fa86
> addons/models/0060-hbase_model.json 1d264df4
> addons/models/0080-storm_model.json 25360ff0
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 68da6af1
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java 2de9bdf0
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipDef.java fc820d49
> intg/src/main/java/org/apache/atlas/model/typedef/AtlasRelationshipEndDef.java f80ea895
> intg/src/main/java/org/apache/atlas/type/AtlasEntityType.java e94dd190
> intg/src/main/java/org/apache/atlas/type/AtlasRelationshipType.java 841b66f7
> intg/src/main/java/org/apache/atlas/type/AtlasStructType.java f97d7674
> intg/src/test/java/org/apache/atlas/TestRelationshipUtilsV2.java PRE-CREATION
> intg/src/test/java/org/apache/atlas/TestUtilsV2.java 9774583d
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 6f6d74bc
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java 12e8bb1f
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasGraphUtilsV1.java cd9a47ad
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 3ff6fbef
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java d4fdc257
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java 157f8cd2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java f4257be7
> repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java de8e7ef3
> repository/src/test/java/org/apache/atlas/repository/impexp/ZipFileResourceTestUtils.java d9017319
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreHardDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreSoftDeleteV1Test.java PRE-CREATION
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java 67702231
>
>
> Diff: https://reviews.apache.org/r/60938/diff/2/
>
>
> Testing
> -------
>
> added unit test - AtlasRelationshipStoreV1Test
>
> mvn clean package - succeeded with no errors.
>
>
> Thanks,
>
> Sarath Subramanian
>
>