----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62681/#review186617 -----------------------------------------------------------
I would have liked more unit tests, since this is core functionality. notification/src/main/java/org/apache/atlas/notification/AbstractNotification.java Line 133 (original), 152 (patched) <https://reviews.apache.org/r/62681/#comment263312> I would suggest splitting this logic into methods, preferably classes. There is lot of logic that will help if there are tests. notification/src/main/java/org/apache/atlas/notification/AlasNotificationMessageDeserializer.java Lines 78 (patched) <https://reviews.apache.org/r/62681/#comment263313> Recommend splitting this in methods or classes. This logic can thus be testtable. notification/src/main/java/org/apache/atlas/notification/AtlasNotificationBaseMessage.java Lines 36 (patched) <https://reviews.apache.org/r/62681/#comment263315> This class seems to be doing 3 different things. Will help to model it that way. notification/src/main/java/org/apache/atlas/notification/AtlasNotificationBaseMessage.java Lines 130 (patched) <https://reviews.apache.org/r/62681/#comment263314> Consider encapsulating these methods into separate utility class. - Ashutosh Mestry On Sept. 28, 2017, 11:08 p.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62681/ > ----------------------------------------------------------- > > (Updated Sept. 28, 2017, 11:08 p.m.) > > > Review request for atlas, Apoorv Naik, Ashutosh Mestry, Abhay Kulkarni, > Sarath Subramanian, and Suma Shivaprasad. > > > Bugs: ATLAS-2075 > https://issues.apache.org/jira/browse/ATLAS-2075 > > > Repository: atlas > > > Description > ------- > > Update notification module to compress and split large messages (> 1mb) that > can't be sent via Kafka. When such a message is sent, the message is first > compressed via GZip. If compressed message is within the size limit, the > message is sent via Kafka. If the compressed size is larger, then the message > is split into multiple small messages and sent via Kafka. > > Notification consumer is updated to merge and uncompress messages before the > message is processed further. > > > Diffs > ----- > > common/src/main/java/org/apache/atlas/AtlasConfiguration.java 9a9bb76a > notification/src/main/java/org/apache/atlas/kafka/AtlasKafkaConsumer.java > d3b4e49e > notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java > 38889ef8 > > notification/src/main/java/org/apache/atlas/notification/AbstractMessageDeserializer.java > ec99372d > > notification/src/main/java/org/apache/atlas/notification/AbstractNotification.java > cb44fc68 > > notification/src/main/java/org/apache/atlas/notification/AlasNotificationMessageDeserializer.java > PRE-CREATION > > notification/src/main/java/org/apache/atlas/notification/AtlasNotificationBaseMessage.java > PRE-CREATION > > notification/src/main/java/org/apache/atlas/notification/AtlasNotificationStringMessage.java > PRE-CREATION > > notification/src/main/java/org/apache/atlas/notification/MessageVersion.java > 6ef407ac > > notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java > 956c85e0 > > notification/src/main/java/org/apache/atlas/notification/VersionedMessage.java > 1929eb46 > > notification/src/main/java/org/apache/atlas/notification/VersionedMessageDeserializer.java > cc2099e7 > notification/src/test/java/org/apache/atlas/kafka/KafkaConsumerTest.java > 9b712f44 > > notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationMockTest.java > b7474a0e > > notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java > 3b2a093b > > notification/src/test/java/org/apache/atlas/notification/AbstractNotificationTest.java > 61107a9f > > notification/src/test/java/org/apache/atlas/notification/VersionedMessageTest.java > 587b7ebd > > notification/src/test/java/org/apache/atlas/notification/entity/EntityMessageDeserializerTest.java > be324277 > > notification/src/test/java/org/apache/atlas/notification/hook/HookMessageDeserializerTest.java > 3724fd5f > > > Diff: https://reviews.apache.org/r/62681/diff/1/ > > > Testing > ------- > > - verified that large messages are compressed and split by Atlas hook, before > sending to Kafka > - verified that Atlas server processes compressed, multi-part messages > correctly > > > Thanks, > > Madhan Neethiraj > >
