You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Graham Wallis <gr...@uk.ibm.com> on 2018/05/04 10:55:45 UTC
Review Request 66949: ATLAS-2523: changes to accept external GUIDs and
manage homeIds
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/
-----------------------------------------------------------
Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
Repository: atlas
Description
-------
ATLAS-2523: Changes to accept external GUIDs and manage homeIds
Diffs
-----
common/src/main/java/org/apache/atlas/repository/Constants.java c570be2ccbc41a426b0f093b4a33163092223f2f
intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java e6a7f19633a1642f6b415db99e20c0641df9463b
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d283fdba90b917f38eba6c937021352df
intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 1b6af94f24eb7d99953f72815167c868a9bd9efd
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java edf10da9533803234342dc3313ea1024c5e7030f
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6ddaa1b74b2c81b980bcb455b614f4a4ed7
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java d51adad80e133d636ef84883d395126ae0150af5
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 3b9a287f6986ff6bca26fc20b2f94ec1700dbe1e
Diff: https://reviews.apache.org/r/66949/diff/1/
Testing
-------
Functional testing of these changes to save reference copies of entities and relationships
Thanks,
Graham Wallis
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Graham Wallis <gr...@uk.ibm.com>.
> On May 7, 2018, 6:29 p.m., Ashutosh Mestry wrote:
> > Ship It!
Thanks Ashutosh - I have made changes in the latest patch to address the raised issues. I don't believe any of the changes make a functional difference to the code.
- Graham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review202564
-----------------------------------------------------------
On May 10, 2018, 1:11 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
>
> (Updated May 10, 2018, 1:11 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 3732556ae
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 7f36a10f5
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d2
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 6c0fdbf36
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 7490a15b1
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6dda
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java cd00639de
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 183a2f6c0
>
>
> Diff: https://reviews.apache.org/r/66949/diff/2/
>
>
> Testing
> -------
>
> Functional testing of these changes to save reference copies of entities and relationships
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Ashutosh Mestry <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review202564
-----------------------------------------------------------
Ship it!
Ship It!
- Ashutosh Mestry
On May 4, 2018, 10:55 a.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
>
> (Updated May 4, 2018, 10:55 a.m.)
>
>
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java c570be2ccbc41a426b0f093b4a33163092223f2f
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java e6a7f19633a1642f6b415db99e20c0641df9463b
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d283fdba90b917f38eba6c937021352df
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 1b6af94f24eb7d99953f72815167c868a9bd9efd
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java edf10da9533803234342dc3313ea1024c5e7030f
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6ddaa1b74b2c81b980bcb455b614f4a4ed7
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java d51adad80e133d636ef84883d395126ae0150af5
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 3b9a287f6986ff6bca26fc20b2f94ec1700dbe1e
>
>
> Diff: https://reviews.apache.org/r/66949/diff/1/
>
>
> Testing
> -------
>
> Functional testing of these changes to save reference copies of entities and relationships
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Graham Wallis <gr...@uk.ibm.com>.
> On May 16, 2018, 4:37 p.m., Madhan Neethiraj wrote:
> > intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java
> > Lines 77 (patched)
> > <https://reviews.apache.org/r/66949/diff/3/?file=2019359#file2019359line77>
> >
> > Shouldn't homeId be added to AtlasObjectId as well? Please review and update.
I'm not sure - the GUID is still globally unique provides a means to uniquely identify an entity or relationship. The homeId is additional context, but it's not clear to me that it would be required in a reference to an entity or relationship. The purpose of homeId is to enable the enterprise OMRS to determine whether an object originated from (is 'owned' by) Atlas or is something a copy stored in Atlas, of an object that originated in a different repository. The enterprise OMRS uses that information for routing, etc.
I am happy to add homeId to AtlasObjectId (and maybe AtlasRelatedObjectId) if you still think it should be there.
- Graham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review203241
-----------------------------------------------------------
On May 10, 2018, 2:52 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
>
> (Updated May 10, 2018, 2:52 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 3732556ae
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 7f36a10f5
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d2
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 6c0fdbf36
> repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java 78ab206d3
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 7490a15b1
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6dda
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java cd00639de
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 183a2f6c0
>
>
> Diff: https://reviews.apache.org/r/66949/diff/3/
>
>
> Testing
> -------
>
> Functional testing of these changes to save reference copies of entities and relationships
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Madhan Neethiraj <ma...@apache.org>.
> On May 16, 2018, 4:37 p.m., Madhan Neethiraj wrote:
> > intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java
> > Lines 77 (patched)
> > <https://reviews.apache.org/r/66949/diff/3/?file=2019359#file2019359line77>
> >
> > Shouldn't homeId be added to AtlasObjectId as well? Please review and update.
>
> Graham Wallis wrote:
> I'm not sure - the GUID is still globally unique provides a means to uniquely identify an entity or relationship. The homeId is additional context, but it's not clear to me that it would be required in a reference to an entity or relationship. The purpose of homeId is to enable the enterprise OMRS to determine whether an object originated from (is 'owned' by) Atlas or is something a copy stored in Atlas, of an object that originated in a different repository. The enterprise OMRS uses that information for routing, etc.
> I am happy to add homeId to AtlasObjectId (and maybe AtlasRelatedObjectId) if you still think it should be there.
If you don't see the need for 'homeId' in AtlasObjectId (and AtlasEntityHeader - which is returned in search results), the current patch is good to go.
Please rebase the patch for current master and commit.
Thanks!
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review203241
-----------------------------------------------------------
On May 10, 2018, 2:52 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
>
> (Updated May 10, 2018, 2:52 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 3732556ae
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 7f36a10f5
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d2
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 6c0fdbf36
> repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java 78ab206d3
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 7490a15b1
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6dda
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java cd00639de
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 183a2f6c0
>
>
> Diff: https://reviews.apache.org/r/66949/diff/3/
>
>
> Testing
> -------
>
> Functional testing of these changes to save reference copies of entities and relationships
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review203241
-----------------------------------------------------------
intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java
Lines 77 (patched)
<https://reviews.apache.org/r/66949/#comment285333>
Shouldn't homeId be added to AtlasObjectId as well? Please review and update.
- Madhan Neethiraj
On May 10, 2018, 2:52 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
>
> (Updated May 10, 2018, 2:52 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 3732556ae
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 7f36a10f5
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d2
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 6c0fdbf36
> repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java 78ab206d3
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 7490a15b1
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6dda
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java cd00639de
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 183a2f6c0
>
>
> Diff: https://reviews.apache.org/r/66949/diff/3/
>
>
> Testing
> -------
>
> Functional testing of these changes to save reference copies of entities and relationships
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review203344
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On May 10, 2018, 2:52 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
>
> (Updated May 10, 2018, 2:52 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 3732556ae
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 7f36a10f5
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d2
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 6c0fdbf36
> repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java 78ab206d3
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 7490a15b1
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6dda
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java cd00639de
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 183a2f6c0
>
>
> Diff: https://reviews.apache.org/r/66949/diff/3/
>
>
> Testing
> -------
>
> Functional testing of these changes to save reference copies of entities and relationships
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Madhan Neethiraj <ma...@apache.org>.
Graham,
Thanks for the quick response. Since there is no need to include this update in 1.0 release, the commit will be pushed only to master branch.
Regards,
Madhan
On 5/25/18, 7:14 AM, "Graham Wallis" <gr...@uk.ibm.com> wrote:
Hi Madhan
I don't mind either way - it should be safe to commit to 1.0, but on the
other hand it is not needed until the Atlas OMRS connector is committed -
which I imagine will be to master and not to 1.0.
It is probably easier to make it master only, just due to not needing to
double fix.
Best regards,
Graham
Graham Wallis
IBM Analytics
Internet: graham_wallis@uk.ibm.com
IBM Laboratories, Hursley Park, Hursley, Hampshire SO21 2JN
Tel: +44 7741 379735
From: Madhan Neethiraj <ma...@apache.org>
To: Graham Wallis <gr...@uk.ibm.com>
Cc: atlas <de...@atlas.incubator.apache.org>, Ashutosh Mestry
<am...@hortonworks.com>
Date: 25/05/2018 14:40
Subject: Re: Review Request 66949: ATLAS-2523: changes to accept
external GUIDs and manage homeIds
Graham,
Please let me know if you need this commit to be included in 1.0 release.
I am planning to send release-candidate for vote later today; and I see no
harm in including this commit.
Thanks,
Madhan
From: Madhan Neethiraj <no...@reviews.apache.org> on behalf of Madhan
Neethiraj <ma...@apache.org>
Reply-To: Madhan Neethiraj <ma...@apache.org>
Date: Friday, May 25, 2018 at 6:37 AM
To: Ashutosh Mestry <am...@hortonworks.com>, Madhan Neethiraj
<ma...@apache.org>
Cc: Graham Wallis <gr...@uk.ibm.com>, atlas
<de...@atlas.incubator.apache.org>
Subject: Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/
Ship it!
Ship It!
- Madhan Neethiraj
On May 25th, 2018, 12:47 p.m. UTC, Graham Wallis wrote:
Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
By Graham Wallis.
Updated May 25, 2018, 12:47 p.m.
Repository: atlas
Description
ATLAS-2523: Changes to accept external GUIDs and manage homeIds
Testing
Functional testing of these changes to save reference copies of entities
and relationships
Diffs
common/src/main/java/org/apache/atlas/repository/Constants.java
(6d95c459a)
intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java
(7f36a10f5)
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java
(debaeeff6)
intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java (6c0fdbf36)
repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java
(78ab206d3)
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java
(3ca287209)
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java
(eb1079c41)
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
(707ea343c)
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
(d5644f95d)
View Diff
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number
741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Review Request 66949: ATLAS-2523: changes to accept external GUIDs and
manage homeIds
Posted by Graham Wallis <gr...@uk.ibm.com>.
Hi Madhan
I don't mind either way - it should be safe to commit to 1.0, but on the
other hand it is not needed until the Atlas OMRS connector is committed -
which I imagine will be to master and not to 1.0.
It is probably easier to make it master only, just due to not needing to
double fix.
Best regards,
Graham
Graham Wallis
IBM Analytics
Internet: graham_wallis@uk.ibm.com
IBM Laboratories, Hursley Park, Hursley, Hampshire SO21 2JN
Tel: +44 7741 379735
From: Madhan Neethiraj <ma...@apache.org>
To: Graham Wallis <gr...@uk.ibm.com>
Cc: atlas <de...@atlas.incubator.apache.org>, Ashutosh Mestry
<am...@hortonworks.com>
Date: 25/05/2018 14:40
Subject: Re: Review Request 66949: ATLAS-2523: changes to accept
external GUIDs and manage homeIds
Graham,
Please let me know if you need this commit to be included in 1.0 release.
I am planning to send release-candidate for vote later today; and I see no
harm in including this commit.
Thanks,
Madhan
From: Madhan Neethiraj <no...@reviews.apache.org> on behalf of Madhan
Neethiraj <ma...@apache.org>
Reply-To: Madhan Neethiraj <ma...@apache.org>
Date: Friday, May 25, 2018 at 6:37 AM
To: Ashutosh Mestry <am...@hortonworks.com>, Madhan Neethiraj
<ma...@apache.org>
Cc: Graham Wallis <gr...@uk.ibm.com>, atlas
<de...@atlas.incubator.apache.org>
Subject: Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/
Ship it!
Ship It!
- Madhan Neethiraj
On May 25th, 2018, 12:47 p.m. UTC, Graham Wallis wrote:
Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
By Graham Wallis.
Updated May 25, 2018, 12:47 p.m.
Repository: atlas
Description
ATLAS-2523: Changes to accept external GUIDs and manage homeIds
Testing
Functional testing of these changes to save reference copies of entities
and relationships
Diffs
common/src/main/java/org/apache/atlas/repository/Constants.java
(6d95c459a)
intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java
(7f36a10f5)
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java
(debaeeff6)
intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java (6c0fdbf36)
repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java
(78ab206d3)
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java
(3ca287209)
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java
(eb1079c41)
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java
(707ea343c)
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java
(d5644f95d)
View Diff
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number
741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Madhan Neethiraj <ma...@apache.org>.
Graham,
Please let me know if you need this commit to be included in 1.0 release. I am planning to send release-candidate for vote later today; and I see no harm in including this commit.
Thanks,
Madhan
From: Madhan Neethiraj <no...@reviews.apache.org> on behalf of Madhan Neethiraj <ma...@apache.org>
Reply-To: Madhan Neethiraj <ma...@apache.org>
Date: Friday, May 25, 2018 at 6:37 AM
To: Ashutosh Mestry <am...@hortonworks.com>, Madhan Neethiraj <ma...@apache.org>
Cc: Graham Wallis <gr...@uk.ibm.com>, atlas <de...@atlas.incubator.apache.org>
Subject: Re: Review Request 66949: ATLAS-2523: changes to accept external GUIDs and manage homeIds
This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66949/
Ship it!
Ship It!
- Madhan Neethiraj
On May 25th, 2018, 12:47 p.m. UTC, Graham Wallis wrote:
Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
By Graham Wallis.
Updated May 25, 2018, 12:47 p.m.Repository: atlas
Description
ATLAS-2523: Changes to accept external GUIDs and manage homeIds
Testing
Functional testing of these changes to save reference copies of entities and relationships
Diffs
common/src/main/java/org/apache/atlas/repository/Constants.java (6d95c459a)
intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java (7f36a10f5)
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java (debaeeff6)
intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java (6c0fdbf36)
repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java (78ab206d3)
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java (3ca287209)
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java (eb1079c41)
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java (707ea343c)
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java (d5644f95d)
View Diff
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review203874
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On May 25, 2018, 12:47 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
>
> (Updated May 25, 2018, 12:47 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 6d95c459a
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 7f36a10f5
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java debaeeff6
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 6c0fdbf36
> repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java 78ab206d3
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 3ca287209
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java eb1079c41
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java 707ea343c
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java d5644f95d
>
>
> Diff: https://reviews.apache.org/r/66949/diff/4/
>
>
> Testing
> -------
>
> Functional testing of these changes to save reference copies of entities and relationships
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Graham Wallis <gr...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/
-----------------------------------------------------------
(Updated May 25, 2018, 12:47 p.m.)
Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
Changes
-------
Re-based on current master
Repository: atlas
Description
-------
ATLAS-2523: Changes to accept external GUIDs and manage homeIds
Diffs (updated)
-----
common/src/main/java/org/apache/atlas/repository/Constants.java 6d95c459a
intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 7f36a10f5
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java debaeeff6
intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 6c0fdbf36
repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java 78ab206d3
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 3ca287209
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java eb1079c41
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphMapper.java 707ea343c
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/EntityGraphRetriever.java d5644f95d
Diff: https://reviews.apache.org/r/66949/diff/4/
Changes: https://reviews.apache.org/r/66949/diff/3-4/
Testing
-------
Functional testing of these changes to save reference copies of entities and relationships
Thanks,
Graham Wallis
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Graham Wallis <gr...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/
-----------------------------------------------------------
(Updated May 10, 2018, 2:52 p.m.)
Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
Changes
-------
Added registry based lookup
Repository: atlas
Description
-------
ATLAS-2523: Changes to accept external GUIDs and manage homeIds
Diffs (updated)
-----
common/src/main/java/org/apache/atlas/repository/Constants.java 3732556ae
intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 7f36a10f5
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d2
intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 6c0fdbf36
repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java 78ab206d3
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 7490a15b1
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6dda
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java cd00639de
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 183a2f6c0
Diff: https://reviews.apache.org/r/66949/diff/3/
Changes: https://reviews.apache.org/r/66949/diff/2-3/
Testing
-------
Functional testing of these changes to save reference copies of entities and relationships
Thanks,
Graham Wallis
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Graham Wallis <gr...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/
-----------------------------------------------------------
(Updated May 10, 2018, 1:11 p.m.)
Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
Changes
-------
Refreshed patch - to address earlier review issues
Repository: atlas
Description
-------
ATLAS-2523: Changes to accept external GUIDs and manage homeIds
Diffs (updated)
-----
common/src/main/java/org/apache/atlas/repository/Constants.java 3732556ae
intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 7f36a10f5
intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d2
intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 6c0fdbf36
repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 7490a15b1
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6dda
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java cd00639de
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 183a2f6c0
Diff: https://reviews.apache.org/r/66949/diff/2/
Changes: https://reviews.apache.org/r/66949/diff/1-2/
Testing
-------
Functional testing of these changes to save reference copies of entities and relationships
Thanks,
Graham Wallis
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Graham Wallis <gr...@uk.ibm.com>.
> On May 4, 2018, 12:04 p.m., David Radley wrote:
> > common/src/main/java/org/apache/atlas/repository/Constants.java
> > Lines 80 (patched)
> > <https://reviews.apache.org/r/66949/diff/1/?file=2016731#file2016731line80>
> >
> > I suggest a comment here to describe how an Atlas developer should understand this field.
> >
> > A concern I have, is that if the users go through the om aPI then the entities with a non-null homId will never be updated - apart from as part of the asynchronous cnotification process from anotehr server. The Atlas developer using the local Atlas API, needs to be aware not to change the entities and relationships with a none null homeId. Ideally we should police this.
I can certainly add a comment to say what the field is for. I see your point about the policing of updates but I might suggest that should be a separate JIRA.
> On May 4, 2018, 12:04 p.m., David Radley wrote:
> > intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java
> > Lines 61 (patched)
> > <https://reviews.apache.org/r/66949/diff/1/?file=2016734#file2016734line61>
> >
> > I am wondering why you are adding a Logger and not using it om this change.
I had debug statements in the code during development and testing; they have been removed for brevity but I forgot to clear out the logger.
> On May 4, 2018, 12:04 p.m., David Radley wrote:
> > intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java
> > Line 384 (original), 389 (patched)
> > <https://reviews.apache.org/r/66949/diff/1/?file=2016734#file2016734line389>
> >
> > I suggest adding a comment here, around why you are checking for '-' as the first character.
> >
> > What happens if another repository creates an identifier starting with a '-' character?
> >
> > to cope with theis case - I am suggest that the logic should check for a null homeId and a guid starting character of '-' (or null) we should say the guid is not assigned.
I like that - I wrote the GUID handling change before adding the homeId system attribute. We could (now) include a check for homeId (not null) in the GUID validity checking.
> On May 4, 2018, 12:04 p.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
> > Line 709 (original), 709 (patched)
> > <https://reviews.apache.org/r/66949/diff/1/?file=2016736#file2016736line709>
> >
> > why are we leaving in commented out code
No valid reason - I wanted to highlight the effect of the change by having the 'before' and 'after' - but if the patch is seen as generally OK I should remove it.
- Graham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review202433
-----------------------------------------------------------
On May 4, 2018, 10:55 a.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
>
> (Updated May 4, 2018, 10:55 a.m.)
>
>
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java c570be2ccbc41a426b0f093b4a33163092223f2f
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java e6a7f19633a1642f6b415db99e20c0641df9463b
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d283fdba90b917f38eba6c937021352df
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 1b6af94f24eb7d99953f72815167c868a9bd9efd
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java edf10da9533803234342dc3313ea1024c5e7030f
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6ddaa1b74b2c81b980bcb455b614f4a4ed7
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java d51adad80e133d636ef84883d395126ae0150af5
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 3b9a287f6986ff6bca26fc20b2f94ec1700dbe1e
>
>
> Diff: https://reviews.apache.org/r/66949/diff/1/
>
>
> Testing
> -------
>
> Functional testing of these changes to save reference copies of entities and relationships
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by Graham Wallis <gr...@uk.ibm.com>.
> On May 4, 2018, 12:04 p.m., David Radley wrote:
> > intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java
> > Line 384 (original), 389 (patched)
> > <https://reviews.apache.org/r/66949/diff/1/?file=2016734#file2016734line389>
> >
> > I suggest adding a comment here, around why you are checking for '-' as the first character.
> >
> > What happens if another repository creates an identifier starting with a '-' character?
> >
> > to cope with theis case - I am suggest that the logic should check for a null homeId and a guid starting character of '-' (or null) we should say the guid is not assigned.
>
> Graham Wallis wrote:
> I like that - I wrote the GUID handling change before adding the homeId system attribute. We could (now) include a check for homeId (not null) in the GUID validity checking.
Actually when I came to implement this I discovered that it would require a pervasive change to Atlas (for example, isAssignedGuid would need to accept the homeId for any objects that possess it, which would affect all the callers of isAssignedGuid). So instead I have added a comment explaining the current checking and that it should be enhanced only if the requirement arises to support objects from a repository that assigns GUIDs prefixed with the '-' character. If such a requirement arises it might alternatively be possible to solve the requirement in the OMRS layer rather than inside Atlas - this could be evaluated at the time.
- Graham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review202433
-----------------------------------------------------------
On May 10, 2018, 1:11 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
>
> (Updated May 10, 2018, 1:11 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java 3732556ae
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java 7f36a10f5
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d2
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 6c0fdbf36
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 7490a15b1
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6dda
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java cd00639de
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 183a2f6c0
>
>
> Diff: https://reviews.apache.org/r/66949/diff/2/
>
>
> Testing
> -------
>
> Functional testing of these changes to save reference copies of entities and relationships
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 66949: ATLAS-2523: changes to accept external
GUIDs and manage homeIds
Posted by David Radley <da...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66949/#review202433
-----------------------------------------------------------
common/src/main/java/org/apache/atlas/repository/Constants.java
Lines 80 (patched)
<https://reviews.apache.org/r/66949/#comment284251>
I suggest a comment here to describe how an Atlas developer should understand this field.
A concern I have, is that if the users go through the om aPI then the entities with a non-null homId will never be updated - apart from as part of the asynchronous cnotification process from anotehr server. The Atlas developer using the local Atlas API, needs to be aware not to change the entities and relationships with a none null homeId. Ideally we should police this.
intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java
Lines 61 (patched)
<https://reviews.apache.org/r/66949/#comment284248>
I am wondering why you are adding a Logger and not using it om this change.
intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java
Line 384 (original), 389 (patched)
<https://reviews.apache.org/r/66949/#comment284249>
I suggest adding a comment here, around why you are checking for '-' as the first character.
What happens if another repository creates an identifier starting with a '-' character?
to cope with theis case - I am suggest that the logic should check for a null homeId and a guid starting character of '-' (or null) we should say the guid is not assigned.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java
Line 709 (original), 709 (patched)
<https://reviews.apache.org/r/66949/#comment284250>
why are we leaving in commented out code
- David Radley
On May 4, 2018, 10:55 a.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66949/
> -----------------------------------------------------------
>
> (Updated May 4, 2018, 10:55 a.m.)
>
>
> Review request for atlas, Ashutosh Mestry and Madhan Neethiraj.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2523: Changes to accept external GUIDs and manage homeIds
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/repository/Constants.java c570be2ccbc41a426b0f093b4a33163092223f2f
> intg/src/main/java/org/apache/atlas/model/instance/AtlasEntity.java e6a7f19633a1642f6b415db99e20c0641df9463b
> intg/src/main/java/org/apache/atlas/model/instance/AtlasRelationship.java d04daa5d283fdba90b917f38eba6c937021352df
> intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java 1b6af94f24eb7d99953f72815167c868a9bd9efd
> repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java edf10da9533803234342dc3313ea1024c5e7030f
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1.java 9fcba6ddaa1b74b2c81b980bcb455b614f4a4ed7
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java d51adad80e133d636ef84883d395126ae0150af5
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphRetriever.java 3b9a287f6986ff6bca26fc20b2f94ec1700dbe1e
>
>
> Diff: https://reviews.apache.org/r/66949/diff/1/
>
>
> Testing
> -------
>
> Functional testing of these changes to save reference copies of entities and relationships
>
>
> Thanks,
>
> Graham Wallis
>
>