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 Tue, 15 Sep 2015 21:46:25 GMT


> On Sept. 14, 2015, 12:55 p.m., Shwetha GS wrote:
> >

Thanks for the review!


> On Sept. 14, 2015, 12:55 p.m., Shwetha GS wrote:
> > notification/src/main/java/org/apache/atlas/notification/entity/Entity.java, line
25
> > <https://reviews.apache.org/r/38341/diff/1/?file=1071265#file1071265line25>
> >
> >     Users are already exposed to the type system and its classes. Referenceable
in this case. We can use that itself instead of adding another interface

It wasn’t clear to me that IReferenceableInstance was supposed to be end user facing.  It
looks like it’s heavily used internally.  I think that having the notification Entity interface
shields the notification users from any changes that we may need to make to the type system
in the future.  Also, it allows us to expose things in a different way if so required by the
notification users.  One example is the case where the Ranger guys want to see which values
of a entity’s trait belong to the trait’s super types.  You can’t get that through IReferenceableInstance
and IStruct.  It requires an additional call or calls to the type system to get the trait
hierarchy.  If we are trying to make the system easy to use, that’s additional work that
we can do for them.


> On Sept. 14, 2015, 12:55 p.m., Shwetha GS wrote:
> > repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java,
line 469
> > <https://reviews.apache.org/r/38341/diff/1/?file=1071277#file1071277line469>
> >
> >     Move to another file

Could you explain why?  The listener implementation is internal to DefaultMetadataService.
 It's not ever intended to be used outside of this class.


> On Sept. 14, 2015, 12:55 p.m., Shwetha GS wrote:
> > repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java,
line 473
> > <https://reviews.apache.org/r/38341/diff/1/?file=1071277#file1071277line473>
> >
> >     Everywhere else, jettison json is used. Lets re-use it

In that code I just want to marshal a Java object to JSON.  I'm not sure how to do that easily
in Jettison.  Can you tell me how?  See this...  http://stackoverflow.com/questions/15426641/how-to-marshall-pojo-to-json-using-jettison


- Tom


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


On Sept. 14, 2015, 4:08 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38341/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2015, 4:08 a.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/src/main/java/org/apache/atlas/notification/entity/Entity.java PRE-CREATION

>   notification/src/main/java/org/apache/atlas/notification/entity/EntityChangeConsumer.java
PRE-CREATION 
>   notification/src/main/java/org/apache/atlas/notification/entity/EntityChangeListener.java
PRE-CREATION 
>   notification/src/main/java/org/apache/atlas/notification/entity/EntityImpl.java PRE-CREATION

>   notification/src/main/java/org/apache/atlas/notification/entity/EntityNotification.java
PRE-CREATION 
>   notification/src/main/java/org/apache/atlas/notification/entity/Trait.java PRE-CREATION

>   notification/src/main/java/org/apache/atlas/notification/entity/TraitImpl.java PRE-CREATION

>   notification/src/test/java/org/apache/atlas/notification/entity/EntityImplTest.java
PRE-CREATION 
>   notification/src/test/java/org/apache/atlas/notification/entity/TraitImplTest.java
PRE-CREATION 
>   repository/pom.xml 8e4d0f3 
>   repository/src/main/java/org/apache/atlas/RepositoryMetadataModule.java fbd01de 
>   repository/src/main/java/org/apache/atlas/listener/EntityChangeListener.java f58d6de

>   repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java 56168db

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