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

Reply via email to