> On Sept. 7, 2015, 4:51 a.m., Pallavi Rao wrote: > > client/src/main/resources/feed-0.1.xsd, line 117 > > <https://reviews.apache.org/r/38105/diff/2/?file=1064612#file1064612line117> > > > > Add doc here as you have done in process xsd.
done. > On Sept. 7, 2015, 4:51 a.m., Pallavi Rao wrote: > > prism/src/main/java/org/apache/falcon/plugin/EmailNotification.java, line 43 > > <https://reviews.apache.org/r/38105/diff/2/?file=1064626#file1064626line43> > > > > This class and other classes following this are completely missing UTs. > > Can we add some UTs, please? UT's added for class EmailNotification using GreenEmail server. > On Sept. 7, 2015, 4:51 a.m., Pallavi Rao wrote: > > metrics/src/main/java/org/apache/falcon/util/EmailNotificationArgs.java, > > line 28 > > <https://reviews.apache.org/r/38105/diff/2/?file=1064619#file1064619line28> > > > > Nit : Add to startup.properties as a commented line, so users know the > > arg to set. added the auth arg in statup.properties. Also done changes in class EmailNotification to handle auth. > On Sept. 7, 2015, 4:51 a.m., Pallavi Rao wrote: > > oozie/src/test/java/org/apache/falcon/oozie/feed/OozieFeedWorkflowBuilderTest.java, > > line 287 > > <https://reviews.apache.org/r/38105/diff/2/?file=1064621#file1064621line287> > > > > Nit : Rename the method to reflect what it is doing.. > > "testEmailNotification" should suffice. done > On Sept. 7, 2015, 4:51 a.m., Pallavi Rao wrote: > > metrics/pom.xml, line 66 > > <https://reviews.apache.org/r/38105/diff/2/?file=1064617#file1064617line66> > > > > Why 1.4 when 1.4.7 is the latest? done > On Sept. 7, 2015, 4:51 a.m., Pallavi Rao wrote: > > client/src/main/resources/feed-0.1.xsd, line 297 > > <https://reviews.apache.org/r/38105/diff/2/?file=1064612#file1064612line297> > > > > I see that you have kept the notification generic (it can be http > > tomorrow). In that case, should we call this "destination" instead? I am of the opinion to keep it "to" only. for e.g: to:<email address>, to:<kafka topic>, to:<http url> . - Peeyush ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38105/#review97889 ----------------------------------------------------------- On Sept. 5, 2015, 12:12 p.m., Peeyush Bishnoi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38105/ > ----------------------------------------------------------- > > (Updated Sept. 5, 2015, 12:12 p.m.) > > > Review request for Falcon. > > > Bugs: FALCON-1425 > https://issues.apache.org/jira/browse/FALCON-1425 > > > Repository: falcon-git > > > Description > ------- > > Provide Email based notification plugin to send notification when Falcon > instance completes. > > > Diffs > ----- > > client/src/main/java/org/apache/falcon/entity/v0/EntityNotification.java > PRE-CREATION > client/src/main/resources/feed-0.1.xsd 4ff8baa > client/src/main/resources/jaxb-binding.xjb f644f40 > client/src/main/resources/process-0.1.xsd c81d6f7 > common/src/main/java/org/apache/falcon/entity/EntityUtil.java 25d9008 > common/src/main/resources/startup.properties c48188c > metrics/pom.xml 748fb97 > metrics/src/main/java/org/apache/falcon/plugin/NotificationPlugin.java > PRE-CREATION > metrics/src/main/java/org/apache/falcon/util/EmailNotificationArgs.java > PRE-CREATION > metrics/src/main/java/org/apache/falcon/util/NotificationType.java > PRE-CREATION > > oozie/src/test/java/org/apache/falcon/oozie/feed/OozieFeedWorkflowBuilderTest.java > 2f7787d > > oozie/src/test/java/org/apache/falcon/oozie/process/OozieProcessWorkflowBuilderTest.java > d727ca3 > oozie/src/test/resources/config/process/process-notification-0.1.xml > PRE-CREATION > oozie/src/test/resources/feed/feed-notification.xml PRE-CREATION > prism/pom.xml be04ac9 > prism/src/main/java/org/apache/falcon/plugin/EmailNotification.java > PRE-CREATION > prism/src/main/java/org/apache/falcon/plugin/EmailNotificationPlugin.java > PRE-CREATION > prism/src/main/java/org/apache/falcon/plugin/NotificationHandler.java > PRE-CREATION > prism/src/main/java/org/apache/falcon/util/NotificationUtil.java > PRE-CREATION > src/conf/startup.properties 9925373 > > Diff: https://reviews.apache.org/r/38105/diff/ > > > Testing > ------- > > Yes, manual testing has been done for this after configuring > startup.properties with SMTP properties. > Also test cases has been added to test Falcon feed/process entity with > notification tag. > > > Thanks, > > Peeyush Bishnoi > >
