atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tom Beerbower" <tbeerbo...@hortonworks.com>
Subject Re: Review Request 38341: Provide Entity Change Notification
Date Mon, 05 Oct 2015 16:58:53 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message