> On April 25, 2016, 7:10 a.m., Shwetha GS wrote:
> > distro/src/conf/atlas-log4j.xml, line 57
> > <https://reviews.apache.org/r/45720/diff/6/?file=1353178#file1353178line57>
> >
> >     These logs are created at client space and we shouldn't force them to 
> > create these loggers. Lets use default loggers. 
> >     
> >     For any incompatible messages in hook notifications, we can use this 
> > logger(after catching the IncompatibleVersionException as atlas is the 
> > client in this case)
> 
> Tom Beerbower wrote:
>     Could you explain more what the concern is?  There is a requirement to 
> log the incompatible messages to a different file.

Tom, Shwetha discussed this point with me. The point of contention was that how 
to handle an incompatible message (where handling is logging to a file, doing 
something else) is the decision of the consumer. For e.g. Ranger which consumes 
the ATLAS_ENTITIES message in a HDP stack may not want to log messages, but 
just throw up etc. Hence, leaving the choice of logging to the consumer seems a 
good (more flexible) design choice. From that perspective, we could just throw 
the incompatible message exception with the message. Logging incompatible 
ATLAS_HOOK messages was a choice we took as consumers, hence we could move the 
logging to the Notification Consumer code.

This does mean we need to document the exception details and let consumers know 
about this so they can handle it appropriately.


- Hemanth


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


On April 20, 2016, 1:26 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45720/
> -----------------------------------------------------------
> 
> (Updated April 20, 2016, 1:26 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Bugs: ATLAS-631
>     https://issues.apache.org/jira/browse/ATLAS-631
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> 1. Introduce Versioning to Atlas Notification Payload (both ways)
> 2. For any messages that are not able to be processed, log the message do a 
> separate log file for unprocessed messages.
> 
> 
> Diffs
> -----
> 
>   distro/src/conf/atlas-log4j.xml 1ac4082 
>   notification/src/main/java/org/apache/atlas/kafka/KafkaConsumer.java 
> 029a072 
>   notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 
> 889af11 
>   
> notification/src/main/java/org/apache/atlas/notification/AbstractMessageDeserializer.java
>  PRE-CREATION 
>   
> notification/src/main/java/org/apache/atlas/notification/AbstractNotification.java
>  596f988 
>   
> notification/src/main/java/org/apache/atlas/notification/AbstractNotificationConsumer.java
>  1cadb99 
>   
> notification/src/main/java/org/apache/atlas/notification/IncompatibleVersionException.java
>  PRE-CREATION 
>   
> notification/src/main/java/org/apache/atlas/notification/MessageDeserializer.java
>  PRE-CREATION 
>   
> notification/src/main/java/org/apache/atlas/notification/MessageVersion.java 
> PRE-CREATION 
>   
> notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java
>  ac285aa 
>   
> notification/src/main/java/org/apache/atlas/notification/VersionedMessage.java
>  PRE-CREATION 
>   
> notification/src/main/java/org/apache/atlas/notification/VersionedMessageDeserializer.java
>  PRE-CREATION 
>   
> notification/src/main/java/org/apache/atlas/notification/entity/EntityMessageDeserializer.java
>  PRE-CREATION 
>   
> notification/src/main/java/org/apache/atlas/notification/hook/HookMessageDeserializer.java
>  PRE-CREATION 
>   notification/src/test/java/org/apache/atlas/kafka/KafkaConsumerTest.java 
> PRE-CREATION 
>   
> notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationTest.java 
> db34815 
>   
> notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java
>  PRE-CREATION 
>   
> notification/src/test/java/org/apache/atlas/notification/AbstractNotificationTest.java
>  PRE-CREATION 
>   
> notification/src/test/java/org/apache/atlas/notification/MessageVersionTest.java
>  PRE-CREATION 
>   
> notification/src/test/java/org/apache/atlas/notification/VersionedMessageTest.java
>  PRE-CREATION 
>   
> notification/src/test/java/org/apache/atlas/notification/entity/EntityMessageDeserializerTest.java
>  PRE-CREATION 
>   
> notification/src/test/java/org/apache/atlas/notification/entity/EntityNotificationImplTest.java
>  385c41f 
>   
> notification/src/test/java/org/apache/atlas/notification/hook/HookMessageDeserializerTest.java
>  PRE-CREATION 
>   
> notification/src/test/java/org/apache/atlas/notification/hook/HookNotificationTest.java
>  57b0eea 
>   typesystem/src/main/resources/atlas-log4j.xml 2bb49d3 
> 
> Diff: https://reviews.apache.org/r/45720/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added.
> 
> mvn clean test.
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>

Reply via email to