Mailing-List: contact dev-help@atlas.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@atlas.incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============6647079151268651267=="
MIME-Version: 1.0
Subject: Re: Review Request 45720: Introduce Versioning to Atlas Notification
 Payload
From: Tom Beerbower <tbeerbower@hortonworks.com>
To: Tom Beerbower <tbeerbower@hortonworks.com>,
 Hemanth Yamijala <yhemanth@gmail.com>, atlas <dev@atlas.incubator.apache.org>,
 Shwetha GS <sshivalingamurthy@hortonworks.com>
Date: Tue, 03 May 2016 13:27:11 -0000
Message-ID: <20160503132711.19046.26689@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: Tom Beerbower <noreply@reviews.apache.org>
References: <20160503044244.19045.87704@reviews.apache.org>
In-Reply-To: <20160503044244.19045.87704@reviews.apache.org>
Reply-To: Tom Beerbower <tbeerbower@hortonworks.com>
archived-at: Tue, 03 May 2016 13:27:26 -0000

--===============6647079151268651267==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On May 3, 2016, 4:42 a.m., Shwetha GS wrote:
> > Build failed with checkstyle errors:
> > [INFO] --- maven-checkstyle-plugin:2.9.1:check (checkstyle-check) @ atlas-notification ---
> > [INFO] Starting audit...
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java:177: Line is longer than 120 characters.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java:250: Line is longer than 120 characters.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/notification/hook/HookMessageDeserializer.java:0: File does not end with a newline.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/notification/MessageVersion.java:27:66: '>' is followed by an illegal character.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/notification/MessageVersion.java:58:11: 'if' is not followed by whitespace.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/notification/MessageVersion.java:71:15: 'if' is not followed by whitespace.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java:46: Line is longer than 120 characters.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java:52: Line is longer than 120 characters.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/notification/VersionedMessage.java:61:46: 'version' hides a field.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/notification/VersionedMessageDeserializer.java:33: Line is longer than 120 characters.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/main/java/org/apache/atlas/notification/VersionedMessageDeserializer.java:99: Line is longer than 120 characters.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/kafka/KafkaConsumerTest.java:0: File does not end with a newline.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/kafka/KafkaConsumerTest.java:86:17: Name 'testNext_versionMismatch' must match pattern '^[a-z][a-zA-Z0-9]*$'.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java:0: File does not end with a newline.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java:82:17: Name 'testNext_backVersion' must match pattern '^[a-z][a-zA-Z0-9]*$'.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java:129:17: Name 'testNext_forwardVersion' must match pattern '^[a-z][a-zA-Z0-9]*$'.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java:213: 'if' construct must use '{}'s.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java:214: 'if' construct must use '{}'s.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java:254: Class TestDeserializer should be declared as final.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/AbstractNotificationTest.java:0: File does not end with a newline.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/AbstractNotificationTest.java:104:54: 'type' hides a field.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/AbstractNotificationTest.java:104:69: 'messages' hides a field.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/entity/EntityMessageDeserializerTest.java:0: File does not end with a newline.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/entity/EntityMessageDeserializerTest.java:58: Line is longer than 120 characters.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/hook/HookMessageDeserializerTest.java:0: File does not end with a newline.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/hook/HookNotificationTest.java:41: Line is longer than 120 characters.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/MessageVersionTest.java:0: File does not end with a newline.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/MessageVersionTest.java:23:8: Unused import - java.util.ArrayList.
> > /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/VersionedMessageTest.java:0: File does not end with a newline.
> > Audit done.
> > 
> > [INFO] There are 29 checkstyle errors.
> > 
> > [INFO] ------------------------------------------------------------------------
> > [INFO] Reactor Summary:
> > [INFO]
> > [INFO] apache-atlas ....................................... SUCCESS [ 10.999 s]
> > [INFO] Apache Atlas Common ................................ SUCCESS [ 15.599 s]
> > [INFO] Apache Atlas Typesystem ............................ SUCCESS [ 57.676 s]
> > [INFO] Apache Atlas Client ................................ SUCCESS [ 17.400 s]
> > [INFO] Apache Atlas Server API ............................ SUCCESS [  8.209 s]
> > [INFO] Apache Atlas Notification .......................... FAILURE [ 21.228 s]
> > [INFO] Apache Atlas Titan ................................. SKIPPED
> > [INFO] Apache Atlas Repository ............................ SKIPPED
> > [INFO] Apache Atlas UI .................................... SKIPPED
> > [INFO] Apache Atlas Web Application ....................... SKIPPED
> > [INFO] Apache Atlas Documentation ......................... SKIPPED
> > [INFO] Apache Atlas FileSystem Model ...................... SKIPPED
> > [INFO] Apache Atlas Hive Bridge ........................... SKIPPED
> > [INFO] Apache Atlas Falcon Bridge ......................... SKIPPED
> > [INFO] Apache Atlas Sqoop Bridge .......................... SKIPPED
> > [INFO] Apache Atlas Storm Bridge .......................... SKIPPED
> > [INFO] Apache Atlas Distribution .......................... SKIPPED
> > [INFO] ------------------------------------------------------------------------
> > [INFO] BUILD FAILURE
> > [INFO] ------------------------------------------------------------------------
> > [INFO] Total time: 02:12 min
> > [INFO] Finished at: 2016-05-03T10:08:08+05:30
> > [INFO] Final Memory: 80M/518M
> > [INFO] ------------------------------------------------------------------------
> > [ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.9.1:check (checkstyle-check) on project atlas-notification: You have 29 Checkstyle violations. -> [Help 1]
> 
> Tom Beerbower wrote:
>     Thanks Shwetha.  I think that I've fixed all the checkstyle issues above.  How are you running checkstyle?  If I do 'mvn checkstyle:checkstyle', it always passes with no issues.
> 
> Shwetha GS wrote:
>     'mvn clean install' runs checkstyle as well
> 
> Shwetha GS wrote:
>     Failed again:
>     [INFO] --- maven-checkstyle-plugin:2.9.1:check (checkstyle-check) @ atlas-notification ---
>     [INFO] Starting audit...
>     /Users/sshivalingamurthy/git/atlas-clone/notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java:258:19: 'static' modifier out of order with the JLS suggestions.
>     Audit done.
>     
>     [INFO] There are 1 checkstyle errors.
>     [INFO] ------------------------------------------------------------------------

Okay thanks.  I think that does it.  

Question ... Why is Atlas Notification set to fail on checkstyle violations when almost all of the other Atlas modules ignore them?


- Tom


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


On May 3, 2016, 1:21 p.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45720/
> -----------------------------------------------------------
> 
> (Updated May 3, 2016, 1:21 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
> -----
> 
>   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 
> 
> Diff: https://reviews.apache.org/r/45720/diff/
> 
> 
> Testing
> -------
> 
> New unit tests added.
> 
> mvn clean test.
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


--===============6647079151268651267==--