Re: Review Request 38341: Provide Entity Change Notification

[Tom Beerbower <tb...@hortonworks.com>]

> On Sept. 24, 2015, 10:24 p.m., Seetharam Venkatesh wrote:
> > repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java, line 250
> > <https://reviews.apache.org/r/38341/diff/2/?file=1081953#file1081953line250>
> >
> >     It would be much cleaner to add a [Entity/Type]ChangeListener for this and avoid duplication of code.
> 
> Tom Beerbower wrote:
>     Thanks for the review.  I used the listener approach initially but decided it was less code and basically the same thing to jusr use a private method since it seems that EntityChangeListener use is pretty much restricted to the DefaultMetadataService anyway.  Since it's all internal to the DefaultMetadataService class, it's pretty much just an implementation detail to me.  I can see how it might be useful to have a public EntityChangeListener for notifications if we ever exposed the listener registration outside of the DefaultMetadataService class, which is probably intended at some point I guess.  I'll make the change to use a listener.  Thanks.

Since this is internal and may include additional changes to existing interfaces (MetadataService and EntityChangeListener), I'd like to handle this as part of another merge.  I'll open another Jira to track as part of the same sprint.


- Tom


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


On Sept. 25, 2015, 2:13 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38341/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 2:13 p.m.)
> 
> 
> Review request for atlas, John Speidel and Shwetha GS.
> 
> 
> Bugs: ATLAS-158
>     https://issues.apache.org/jira/browse/ATLAS-158
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Add entity change notification to Atlas based using the existing atlas-notification module.
> 
> First cut at a patch for the Atlas entity change notification.  Note that at a minimum additional unit tests are required.  I'm putting up the review to get some initial feedback.
> 
> 
> Diffs
> -----
> 
>   notification/pom.xml 2e12520 
>   notification/src/main/java/org/apache/atlas/notification/entity/EntityNotification.java PRE-CREATION 
>   notification/src/main/java/org/apache/atlas/notification/entity/EntityNotificationConsumer.java PRE-CREATION 
>   notification/src/main/java/org/apache/atlas/notification/entity/EntityNotificationConsumerProvider.java PRE-CREATION 
>   notification/src/main/java/org/apache/atlas/notification/entity/EntityNotificationImpl.java PRE-CREATION 
>   notification/src/test/java/org/apache/atlas/notification/entity/EntityNotificationConsumerProviderTest.java PRE-CREATION 
>   notification/src/test/java/org/apache/atlas/notification/entity/EntityNotificationConsumerTest.java PRE-CREATION 
>   notification/src/test/java/org/apache/atlas/notification/entity/EntityNotificationImplTest.java PRE-CREATION 
>   repository/pom.xml 8e4d0f3 
>   repository/src/main/java/org/apache/atlas/RepositoryMetadataModule.java 34f2ba3 
>   repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java ea39f92 
>   repository/src/test/java/org/apache/atlas/TestRepositoryMetadataModuleFactory.java PRE-CREATION 
>   repository/src/test/java/org/apache/atlas/discovery/HiveLineageServiceTest.java db51ae5 
>   repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryTest.java bec3067 
>   repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java 3c22815 
>   repository/src/test/java/org/apache/atlas/services/DefaultMetadataServiceTest.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/EntityImpl.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/IdImpl.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/TraitImpl.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/api/Entity.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/api/Id.java PRE-CREATION 
>   typesystem/src/main/java/org/apache/atlas/typesystem/api/Trait.java PRE-CREATION 
>   typesystem/src/test/java/org/apache/atlas/typesystem/EntityImplTest.java PRE-CREATION 
>   typesystem/src/test/java/org/apache/atlas/typesystem/IdImplTest.java PRE-CREATION 
>   typesystem/src/test/java/org/apache/atlas/typesystem/TraitImplTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38341/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> All existing tests pass.
> 
> New unit tests added (more required).
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>