Re: Review Request 68412: ATLAS-1773: Atlas OMRS Repository Connector

2019-01-10 Thread Graham Wallis

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

(Updated Jan. 10, 2019, 4:52 p.m.)


Review request for atlas.


Changes
---

Updated connector and event mapper - please note that this version pre-reqs 
ATLAS-2810 for compile and ATLAS-2939 and ATLAS-2985 for runtime corrections to 
base function


Repository: atlas


Description
---

ATLAS-1773: Atlas OMRS Repository Connector


Diffs (updated)
-

  open-metadata/README.md PRE-CREATION 
  open-metadata/pom.xml PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapperProvider.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasBaseTypeDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasClassificationDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxy.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxyImpl.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/Comparator.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/DSLQueryHelper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/EntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/FamousFive.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/ISpringBridge.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSErrorCode.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSMetadataCollection.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnector.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnectorProvider.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/SpringBridge.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TypeDefsByCategory.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TypeNameUtils.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/admin/server/spring/OpenMetadataAdminResource.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasClassificationDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasEntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasEntityMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasRelationshipDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasRelationshipMapper.java
 PRE-CREATION 
  pom.xml 49fe7de14bf52f9a1de7153eab0484f42e25e37a 
  webapp/pom.xml e1988c34863053489c3331a994b328adc92347b0 
  webapp/src/main/webapp/WEB-INF/openMetadataContext.xml PRE-CREATION 
  webapp/src/main/webapp/WEB-INF/web.xml 
23dc0637a8b521ab89a16ec8b03895cdaf8bc7d8 


Diff: https://reviews.apache.org/r/68412/diff/4/

Changes: 

Re: Review Request 68412: ATLAS-1773: Atlas OMRS Repository Connector

2018-09-04 Thread Graham Wallis

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

(Updated Sept. 4, 2018, 3:16 p.m.)


Review request for atlas.


Changes
---

Latest updates include learning mode for entity refreshes, offset and pageSize 
handling, improved exception handling around use of registry, acceptance of 
null guid on saveRelationshipReferenceCopy


Repository: atlas


Description
---

ATLAS-1773: Atlas OMRS Repository Connector


Diffs (updated)
-

  open-metadata/pom.xml PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapperProvider.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasBaseTypeDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasClassificationDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxy.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxyImpl.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/Comparator.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/DSLQueryHelper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/EntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/FamousFive.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/ISpringBridge.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSErrorCode.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSMetadataCollection.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnector.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnectorProvider.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/SpringBridge.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TypeDefsByCategory.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TypeNameUtils.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/admin/server/spring/OpenMetadataAdminResource.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasClassificationDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasEntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasEntityMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasRelationshipDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasRelationshipMapper.java
 PRE-CREATION 
  pom.xml c60a53b0e 
  webapp/pom.xml 78aa81e80 
  webapp/src/main/webapp/WEB-INF/openMetadataContext.xml PRE-CREATION 
  webapp/src/main/webapp/WEB-INF/web.xml 23dc0637a 


Diff: https://reviews.apache.org/r/68412/diff/3/

Changes: https://reviews.apache.org/r/68412/diff/2-3/


Testing
---

Unit Testing (approx 80 tests) and Functional Testing of major 

Re: Review Request 68412: ATLAS-1773: Atlas OMRS Repository Connector

2018-08-31 Thread Graham Wallis

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

(Updated Aug. 31, 2018, 3:44 p.m.)


Review request for atlas.


Changes
---

Updated diff with changes from first round of review comments plus enhancements 
from the last week


Repository: atlas


Description
---

ATLAS-1773: Atlas OMRS Repository Connector


Diffs (updated)
-

  open-metadata/pom.xml PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapperProvider.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasBaseTypeDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasClassificationDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxy.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxyImpl.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/Comparator.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/DSLQueryHelper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/EntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/FamousFive.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/ISpringBridge.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSErrorCode.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSMetadataCollection.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnector.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnectorProvider.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/SpringBridge.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TypeDefsByCategory.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TypeNameUtils.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/admin/server/spring/OpenMetadataAdminResource.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasClassificationDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasEntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasEntityMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasRelationshipDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasRelationshipMapper.java
 PRE-CREATION 
  pom.xml c60a53b0e 
  webapp/pom.xml 78aa81e80 
  webapp/src/main/webapp/WEB-INF/openMetadataContext.xml PRE-CREATION 
  webapp/src/main/webapp/WEB-INF/web.xml 23dc0637a 


Diff: https://reviews.apache.org/r/68412/diff/2/

Changes: https://reviews.apache.org/r/68412/diff/1-2/


Testing
---

Unit Testing (approx 80 tests) and Functional Testing of major operations 
(creation of types, instances, reference copies and proxies).


Thanks,

Graham Wallis



Re: Review Request 68412: ATLAS-1773: Atlas OMRS Repository Connector

2018-08-30 Thread Graham Wallis


> On Aug. 26, 2018, 5:28 p.m., Madhan Neethiraj wrote:
> > Graham - here are my comments from partial review; if you would like to 
> > discuss any in further detail, please let me knwo. I will continue 
> > reviewing.

Thanks Madhan - I have fixed all but 3 of the issues, which I'm leaving open 
for now. I raised ATLAS-2844 for the eventTime one, need some extra help from 
you on the immutable superclass one, and I need to work out what to do about 
the third one around cardinality.


> On Aug. 26, 2018, 5:28 p.m., Madhan Neethiraj wrote:
> > open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
> > Lines 179 (patched)
> > 
> >
> > Instead of waiting for 1 second before processing every message, it 
> > might be useful to add a retry inside processMessage() - can result in 
> > better message processing throughput.

Thanks for the suggestion - that was actually how I originally implemented it - 
but there is a fundamental problem, which is that the entity change 
notification listener sends the Kafka publish prior to committing the 
transaction. The event mapper doesn't know whether it is seeing current or 
stale state, it would have to assume it is current. The alternative 
implementation (as I have it now) is to sleep briefly prior to each batch of 
messages (rather than individual messages) in order to yield and give the 
entitiy change notifier thread a chance to commit the entity update. This is 
not guaranteed to succeed of course, but the loss of the event would not be a 
show-stopper. The better solution would be that the entity change notification 
listener should really commit the update prior to the delivery of the 
notification. What happens currently is the publish to Kafka and the resulting 
consumption from Kafka and triggering of the event mapper all occur prior to 
the commit (or rollback).


> On Aug. 26, 2018, 5:28 p.m., Madhan Neethiraj wrote:
> > open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
> > Lines 181 (patched)
> > 
> >
> > It is generally not a good idea to catch & ignore 
> > 'InterruptedException'. This can potentially prevent the JVM from shutting 
> > down.

Thanks - I have changed this to insert an error entry in the log and set 
keepOnRunning to false - hence terminating the loop.


> On Aug. 26, 2018, 5:28 p.m., Madhan Neethiraj wrote:
> > open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
> > Lines 359 (patched)
> > 
> >
> > line #351 and #359 will result in retrieving the entity twice from the 
> > store; consider avoiding this duplicate retrieval - for better performance.

Yes - thanks - I am aware of the double retrieve. I have decided to keep it 
that way for now at least. We could change it if there is a performance 
problem, but the current approach avoids adding a public method to the metdata 
collection API that would retrieve the AtlasEntity. The event mapper and 
repository connector are in separate packages so I can't make the methods 
package-private. The other option would be to use a compound (public) method 
that asks both questions in one method call, e.g. something like 
getEntityCharacteristics - which would allow us to test whether it is local (or 
not) and a proxy (or not). I prefer the pair of simpler methods.


> On Aug. 26, 2018, 5:28 p.m., Madhan Neethiraj wrote:
> > open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
> > Lines 367 (patched)
> > 
> >
> > While processing ENTITY_DELETE notificaiton, getByGuid() call on Atlas 
> > would fail if Atlas is configured for hard-delete. Connector should be able 
> > to process this notification only with entity-guid.

I have enhanced the event mapper to find out whether Atlas performing hard or 
soft deletes, and behave appropriately.


> On Aug. 26, 2018, 5:28 p.m., Madhan Neethiraj wrote:
> > open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
> > Lines 389 (patched)
> > 
> >
> > eventTime should come from Atlas in 'notification' - given OMRS 
> > connector might process the message at a much later time (than when the 
> > notification was generated). Consider filing an Atlas JIRA to add timestamp 
> > in EntityNotification.

OK - can do. I have raised ATLAS-2844 and will leave this review comment open. 
The solution to that JIRA will have to decide when the change actually happens 
- it could 

Re: Review Request 68412: ATLAS-1773: Atlas OMRS Repository Connector

2018-08-30 Thread Graham Wallis


> On Aug. 17, 2018, 3:58 p.m., David Radley wrote:
> >

Thanks David - all fixed.


> On Aug. 17, 2018, 3:58 p.m., David Radley wrote:
> > open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
> > Lines 346 (patched)
> > 
> >
> > What is the implication of leaving htis TODO in the code? I suggest at 
> > least there should be a Jira numebr here to track ths issue.

This was a temporary solution prior to the local user id being available in the 
connector. This is now implemented.


- Graham


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


On Aug. 17, 2018, 2:38 p.m., Graham Wallis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68412/
> ---
> 
> (Updated Aug. 17, 2018, 2:38 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> ATLAS-1773: Atlas OMRS Repository Connector
> 
> 
> Diffs
> -
> 
>   open-metadata/pom.xml PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapperProvider.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasBaseTypeDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasClassificationDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxy.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxyImpl.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/Comparator.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/DSLQueryHelper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/EntityDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/FamousFive.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/ISpringBridge.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSErrorCode.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSMetadataCollection.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnector.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnectorProvider.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/SpringBridge.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TypeDefsByCategory.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TypeNameUtils.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/admin/server/spring/OpenMetadataAdminResource.java
>  PRE-CREATION 
>   
> open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasClassificationDefMapper.java
>  PRE-CREATION 
>   
> 

Re: Review Request 68412: ATLAS-1773: Atlas OMRS Repository Connector

2018-08-26 Thread Madhan Neethiraj

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



Graham - here are my comments from partial review; if you would like to discuss 
any in further detail, please let me knwo. I will continue reviewing.


open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
Lines 179 (patched)


Instead of waiting for 1 second before processing every message, it might 
be useful to add a retry inside processMessage() - can result in better message 
processing throughput.



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
Lines 181 (patched)


It is generally not a good idea to catch & ignore 'InterruptedException'. 
This can potentially prevent the JVM from shutting down.



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
Lines 359 (patched)


line #351 and #359 will result in retrieving the entity twice from the 
store; consider avoiding this duplicate retrieval - for better performance.



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
Lines 367 (patched)


While processing ENTITY_DELETE notificaiton, getByGuid() call on Atlas 
would fail if Atlas is configured for hard-delete. Connector should be able to 
process this notification only with entity-guid.



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
Lines 389 (patched)


eventTime should come from Atlas in 'notification' - given OMRS connector 
might process the message at a much later time (than when the notification was 
generated). Consider filing an Atlas JIRA to add timestamp in 
EntityNotification.



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapperProvider.java
Lines 39 (patched)


For 'static final' members, consider using upper class names.



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapperProvider.java
Lines 62 (patched)


It seems super.connectorClassName and super.connectorTypeBean might be set 
only once i.e. not mutable. If yes, consider setting this via a protected 
constructor:

public AtlasOMRSRepositoryEventMapperProvider() {
  super(AtlasOMRSRepositoryEventMapper.class.getName(), 
initConnectorType());
}

private ConnectorType initConnectorType() {
  ConnectorType connectorType = new ConnectorType();

  connectorType.setType(ConnectorType.getConnectorTypeType());
  connectorType.setGUID(connectorTypeGUID);
  connectorType.setQualifiedName(connectorTypeName);
  connectorType.setDisplayName(connectorTypeName);
  connectorType.setDescription(connectorTypeDescription);
  connectorType.setConnectorProviderClassName(this.getClass().getName());

  return connectorType;
}



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeDefMapper.java
Lines 57 (patched)


It seems these members are immutable; if yes, please consider marking them 
as 'final'.



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeDefMapper.java
Lines 60 (patched)


Since this module runs within Atlas, it might be easier and efficient to 
work with AtlasTypeRegistry - which has all types in-memory and in a more 
consumable way (for example, detecting whether a type is primitive or not is as 
simple as:
  attributeType.getTypeCategory() == TypeCategory.PRIMITIVE;

AtlasTypeDefStore would read from store, which can/should be avoided.



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeDefMapper.java
Lines 104 (patched)


For return-type and parameter-types, consider using possible super-most 
type in the hieratchy; in this case replace ArrayList with List.



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeDefMapper.java
Lines 125 (patched)


convertAtlasAttributeDef() can return null. Should 

Re: Review Request 68412: ATLAS-1773: Atlas OMRS Repository Connector

2018-08-17 Thread David Radley

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




open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
Lines 346 (patched)


What is the implication of leaving htis TODO in the code? I suggest at 
least there should be a Jira numebr here to track ths issue.



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
Lines 376 (patched)


siple => simple



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
Lines 425 (patched)


I think this s a duplicate line of the one above



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
Lines 434 (patched)


typo doesn#t



open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasClassificationDefMapper.java
Lines 244 (patched)


typo fial


- David Radley


On Aug. 17, 2018, 2:38 p.m., Graham Wallis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68412/
> ---
> 
> (Updated Aug. 17, 2018, 2:38 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> ATLAS-1773: Atlas OMRS Repository Connector
> 
> 
> Diffs
> -
> 
>   open-metadata/pom.xml PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapperProvider.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasBaseTypeDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasClassificationDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxy.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxyImpl.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/Comparator.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/DSLQueryHelper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/EntityDefMapper.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/FamousFive.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/ISpringBridge.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSErrorCode.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSMetadataCollection.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnector.java
>  PRE-CREATION 
>   
> open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnectorProvider.java
>  PRE-CREATION 
>   
> 

Review Request 68412: ATLAS-1773: Atlas OMRS Repository Connector

2018-08-17 Thread Graham Wallis

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

Review request for atlas.


Repository: atlas


Description
---

ATLAS-1773: Atlas OMRS Repository Connector


Diffs
-

  open-metadata/pom.xml PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/eventmapper/AtlasOMRSRepositoryEventMapperProvider.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasAttributeMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasBaseTypeDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasClassificationDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasEntityMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasRelationshipMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxy.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/AtlasStoresProxyImpl.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/Comparator.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/DSLQueryHelper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/EntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/FamousFive.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/ISpringBridge.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSErrorCode.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSMetadataCollection.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnector.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/LocalAtlasOMRSRepositoryConnectorProvider.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/SpringBridge.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TypeDefsByCategory.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TypeNameUtils.java
 PRE-CREATION 
  
open-metadata/src/main/java/org/apache/atlas/openmetadata/admin/server/spring/OpenMetadataAdminResource.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasClassificationDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasEntityDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasEntityMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasRelationshipDefMapper.java
 PRE-CREATION 
  
open-metadata/src/test/java/org/apache/atlas/openmetadata/adapters/repositoryconnector/TestAtlasRelationshipMapper.java
 PRE-CREATION 
  pom.xml c60a53b0e2e7038b6228e16ceba47a0080d58622 
  webapp/pom.xml 78aa81e8046b676f47d24eff0671225a3c960929 
  webapp/src/main/webapp/WEB-INF/openMetadataContext.xml PRE-CREATION 
  webapp/src/main/webapp/WEB-INF/web.xml 
23dc0637a8b521ab89a16ec8b03895cdaf8bc7d8 


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


Testing
---

Unit Testing (approx 80 tests) and Functional Testing of major operations 
(creation of types, instances, reference copies and proxies).


Thanks,

Graham Wallis