Re: Review Request 66949: ATLAS-2523: changes to accept external GUIDs and manage homeIds

2018-05-25 Thread Graham Wallis 
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_wal...@uk.ibm.com 
IBM Laboratories, Hursley Park, Hursley, Hampshire SO21 2JN
Tel: +44 7741 379735




From:   Madhan Neethiraj <mad...@apache.org>
To: Graham Wallis <graham_wal...@uk.ibm.com>
Cc: atlas <d...@atlas.incubator.apache.org>, Ashutosh Mestry 
<ames...@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 <nore...@reviews.apache.org> on behalf of Madhan 
Neethiraj <mad...@apache.org>
Reply-To: Madhan Neethiraj <mad...@apache.org>
Date: Friday, May 25, 2018 at 6:37 AM
To: Ashutosh Mestry <ames...@hortonworks.com>, Madhan Neethiraj 
<mad...@apache.org>
Cc: Graham Wallis <graham_wal...@uk.ibm.com>, atlas 
<d...@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

2018-05-25 Thread Madhan Neethiraj 
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 <nore...@reviews.apache.org> on behalf of Madhan 
Neethiraj <mad...@apache.org>
Reply-To: Madhan Neethiraj <mad...@apache.org>
Date: Friday, May 25, 2018 at 6:37 AM
To: Ashutosh Mestry <ames...@hortonworks.com>, Madhan Neethiraj 
<mad...@apache.org>
Cc: Graham Wallis <graham_wal...@uk.ibm.com>, atlas 
<d...@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

2018-05-25 Thread Madhan Neethiraj 

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

2018-05-25 Thread Graham Wallis 

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

2018-05-17 Thread Madhan Neethiraj 


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

2018-05-17 Thread Madhan Neethiraj 

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

2018-05-17 Thread Graham Wallis 


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

2018-05-16 Thread Madhan Neethiraj 

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


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

2018-05-10 Thread Graham Wallis 

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

2018-05-10 Thread Graham Wallis 


> 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

2018-05-10 Thread Graham Wallis 


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

2018-05-10 Thread Graham Wallis 

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

2018-05-08 Thread Graham Wallis 


> On May 4, 2018, 12:04 p.m., David Radley wrote:
> > common/src/main/java/org/apache/atlas/repository/Constants.java
> > Lines 80 (patched)
> > 
> >
> > 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)
> > 
> >
> > 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)
> > 
> >
> > 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)
> > 
> >
> > 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

2018-05-07 Thread Ashutosh Mestry 

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

2018-05-04 Thread David Radley 

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


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)


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)


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)


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

Review Request 66949: ATLAS-2523: changes to accept external GUIDs and manage homeIds

2018-05-04 Thread Graham Wallis 

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