----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10199/#review19415 -----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java <https://reviews.apache.org/r/10199/#comment40147> if this happens, an email will never be sent. Shoudn't we fail the listener if a bad from address is configured? trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java <https://reviews.apache.org/r/10199/#comment40148> Like the JMSJobEventListener, we can combine all this methods in one as the logic is same for any event. trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java <https://reviews.apache.org/r/10199/#comment40150> if you throw this excpetion, wont the caller thread fail sending other events if a thread was supposed to batch events? Seems better to just log and die trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java <https://reviews.apache.org/r/10199/#comment40152> use stringbuilder as its local variable trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java <https://reviews.apache.org/r/10199/#comment40151> url link for workflow action should be the actionId So event.getJobId() should work, no? trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java <https://reviews.apache.org/r/10199/#comment40153> does the transport API take care of thread safety? trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java <https://reviews.apache.org/r/10199/#comment40155> same question as before, should we throw? trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java <https://reviews.apache.org/r/10199/#comment40176> do we need this timer? Every time before a message is sent, it can be checked whether the address needs to be in blacklist or no. It seems you are already doing this. - Virag Kothari On April 18, 2013, 5:29 p.m., Ryota Egashira wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10199/ > ----------------------------------------------------------- > > (Updated April 18, 2013, 5:29 p.m.) > > > Review request for oozie. > > > Description > ------- > > https://issues.apache.org/jira/browse/OOZIE-1294 > > > This addresses bug OOZIE-1294. > https://issues.apache.org/jira/browse/OOZIE-1294 > > > Diffs > ----- > > > trunk/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java > 1469083 > trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java > PRE-CREATION > > trunk/core/src/test/java/org/apache/oozie/email/TestEmailSLAEventListener.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/10199/diff/ > > > Testing > ------- > > still WIP, especially error handling/test case > > > Thanks, > > Ryota Egashira > >
