Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/ --- Review request for atlas, John Speidel and Shwetha GS. Bugs: ATLAS-158 http

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/#review98834 --- notification/src/main/java/org/apache/atlas/notification/entity/Ent

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/#review98843 --- Ship it! Looks good. Not really an issue with this patch as much as

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/#review98990 --- notification/src/main/java/org/apache/atlas/notification/entity/Ent

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/#review98993 --- notification/src/main/java/org/apache/atlas/notification/entity/Ent

Re: Review Request 38341: Provide Entity Change Notification

> On Sept. 14, 2015, 3:15 p.m., John Speidel wrote: > > Looks good. > > Not really an issue with this patch as much as the existing code, I am a > > little concerned about the use of a single exception type "AtlasException" > > everywhere. > > Using a single exception type results in the loss

Re: Review Request 38341: Provide Entity Change Notification

> On Sept. 15, 2015, 6:44 a.m., Suma Shivaprasad wrote: > > Thanks for the review! > On Sept. 15, 2015, 6:44 a.m., Suma Shivaprasad wrote: > > notification/src/main/java/org/apache/atlas/notification/entity/Entity.java, > > line 26 > >

Re: Review Request 38341: Provide Entity Change Notification

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

Re: Review Request 38341: Provide Entity Change Notification

> On Sept. 15, 2015, 5:44 a.m., Shwetha GS wrote: > > notification/src/main/java/org/apache/atlas/notification/entity/EntityChangeConsumer.java, > > line 41 > > > > > > why expose interface using threadpool and liste

Re: Review Request 38341: Provide Entity Change Notification

> On Sept. 15, 2015, 5:44 a.m., Shwetha GS wrote: > > notification/src/main/java/org/apache/atlas/notification/entity/EntityChangeConsumer.java, > > line 41 > > > > > > why expose interface using threadpool and liste

Re: Review Request 38341: Provide Entity Change Notification

> On Sept. 14, 2015, 12:55 p.m., Shwetha GS wrote: > > notification/src/main/java/org/apache/atlas/notification/entity/Entity.java, > > line 25 > > > > > > Users are already exposed to the type system and its classes

Re: Review Request 38341: Provide Entity Change Notification

> On Sept. 14, 2015, 12:55 p.m., Shwetha GS wrote: > > notification/src/main/java/org/apache/atlas/notification/entity/Entity.java, > > line 25 > > > > > > Users are already exposed to the type system and its classes

Re: Review Request 38341: Provide Entity Change Notification

> On Sept. 15, 2015, 5:44 a.m., Shwetha GS wrote: > > notification/src/main/java/org/apache/atlas/notification/entity/EntityChangeConsumer.java, > > line 41 > > > > > > why expose interface using threadpool and liste

Re: Review Request 38341: Provide Entity Change Notification

> On Sept. 15, 2015, 5:44 a.m., Shwetha GS wrote: > > notification/src/main/java/org/apache/atlas/notification/entity/EntityChangeConsumer.java, > > line 41 > > > > > > why expose interface using threadpool and liste

Re: Review Request 38341: Provide Entity Change Notification

> On Sept. 15, 2015, 5:44 a.m., Shwetha GS wrote: > > notification/src/main/java/org/apache/atlas/notification/entity/EntityChangeConsumer.java, > > line 41 > > > > > > why expose interface using threadpool and liste

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/ --- (Updated Sept. 23, 2015, 3:58 a.m.) Review request for atlas, John Speidel and

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/#review100487 --- repository/src/main/java/org/apache/atlas/services/DefaultMetadata

Re: Review Request 38341: Provide Entity Change Notification

> On Sept. 24, 2015, 10:24 p.m., Seetharam Venkatesh wrote: > > repository/src/main/java/org/apache/atlas/services/DefaultMetadataService.java, > > line 250 > > > > > > It would be much cleaner to add a [Entity/Type

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/ --- (Updated Oct. 5, 2015, 5 p.m.) Review request for atlas, John Speidel and Shwet

Re: Review Request 38341: Provide Entity Change Notification

> On Oct. 12, 2015, 11:06 p.m., Shwetha GS wrote: > > Can you also add docs for this in twiki which is part of code? Sure, can you point me to the docs for notification? I'll add this. - Tom --- This is an automatically generated e-mai

Re: Review Request 38341: Provide Entity Change Notification

> On Oct. 12, 2015, 5:36 a.m., Shwetha GS wrote: > > repository/pom.xml, line 49 > > > > > > Lets avoid adding this dependency. I think this is required for > > notification listener? > > > > Lets define the

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/ --- (Updated Oct. 12, 2015, 4:21 a.m.) Review request for atlas, John Speidel and S

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/ --- (Updated Oct. 12, 2015, 4:25 a.m.) Review request for atlas, John Speidel and S

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/#review102205 --- notification/pom.xml (line 87)

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/#review102308 --- The patch does not apply. Can you re-base as well? Thanks - Shweth

Re: Review Request 38341: Provide Entity Change Notification

> On Oct. 12, 2015, 5:36 a.m., Shwetha GS wrote: > > notification/src/main/java/org/apache/atlas/notification/entity/EntityNotificationConsumer.java, > > line 58 > > > > > > I guess the reason for EntityNotificationC

Re: Review Request 38341: Provide Entity Change Notification

> On Oct. 12, 2015, 5:36 a.m., Shwetha GS wrote: > > notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java, > > line 334 > > > > > > Instead of adding extra condition, always read the consumer id

Re: Review Request 38341: Provide Entity Change Notification

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38341/#review102354 --- Can you also add docs for this in twiki which is part of code? - S