You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Ryota Egashira <eg...@yahoo-inc.com> on 2013/03/29 19:53:56 UTC
Review Request: SLA Email Notification
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/
-----------------------------------------------------------
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/client/src/main/java/org/apache/oozie/client/SLAEvent.java 1462575
trunk/core/pom.xml 1462575
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
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Virag Kothari <vi...@yahoo-inc.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/#review19527
-----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java
<https://reviews.apache.org/r/10199/#comment40303>
Threadlocal session
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java
<https://reviews.apache.org/r/10199/#comment40305>
If user configurable?
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java
<https://reviews.apache.org/r/10199/#comment40304>
use guava cache
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43322>
use AtomicInteger
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43293>
Define all the variables as public static constants in EmailActionExecutor and refer from there.
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43294>
oozie.email.
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43295>
oozie.email.
TIMEOUT_DEFAULT
COUNT_DEFAULT
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43296>
oozie.email.
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43301>
Add onDurationMiss also. And for each event, check the alert-events and don't send mail based on the configured value. If it is null, send alert-events to the values configured in oozie-site.xml. Load those values into a hashset in init() method for checking every time.
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43314>
use getIfPresent and fetch once. And instead of contains do ==null check.
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43319>
Remove the comment
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43321>
Add Parent JobID
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43320>
To be removed
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43323>
Invalid addresses - have a separate cache and blacklist for a day.
trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43324>
Not sure why the format is like this. Haven't reviewed this class yet.
- Rohini Palaniswamy
On May 23, 2013, 8:40 a.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10199/
> -----------------------------------------------------------
>
> (Updated May 23, 2013, 8:40 a.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 1485598
> trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
> trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10199/diff/
>
>
> Testing
> -------
>
> still WIP, especially error handling/test case
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/#review21044
-----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43537>
Apache header
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43529>
Rename it to CONF_ALERT_EVENTS and change the name of setting also in oozie-default.xml
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43530>
Add "Oozie" and appname also in subject
Oozie - SLA START_MISS (AppName=abc, JobID=xyz)
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43531>
Can we change the order slightly with categorization headings (printHeading and printField).
Status:
\tSLA Status -
\tJob Status -
\tNotification Message -
Job Details:
\tApp Name
\tApp Type
\tJob ID
\tJob URL
\tParent Job ID (print only if not null)
\tParent Job URL (if parent job is not null)
\tUser
\tUpstream Apps
SLA Details:
\tNominal Time
\tExpected start
\tActual start
...
...
Actual duration
Use the enum constructor (like EVENT_STATUS("SLA Status")) to pass the text that should appear in the message and use that while constructing the email message body. All Caps text in a mail might will make reading uncomfortable.
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43532>
if (event.getParentId !== null)
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43533>
getJobLink(jobID)
- common method for both jobid and parent id
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43534>
Needs to be getId after the JMS patch goes in
trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43536>
Apache header
trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43538>
Is variable substitution supported in notification message?
- Rohini Palaniswamy
On May 25, 2013, 8:13 a.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10199/
> -----------------------------------------------------------
>
> (Updated May 25, 2013, 8:13 a.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 1486210
> trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
> trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10199/diff/
>
>
> Testing
> -------
>
> local test done
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/#review21131
-----------------------------------------------------------
Ship it!
Ship It!
- Rohini Palaniswamy
On May 28, 2013, 8:20 p.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10199/
> -----------------------------------------------------------
>
> (Updated May 28, 2013, 8:20 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 1487092
> trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
> trunk/core/src/main/java/org/apache/oozie/sla/service/SLAService.java 1487092
> trunk/core/src/main/resources/oozie-default.xml 1487092
> trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10199/diff/
>
>
> Testing
> -------
>
> local test done
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/
-----------------------------------------------------------
(Updated May 28, 2013, 8:20 p.m.)
Review request for oozie.
Changes
-------
-rebased patch and change getJobID->getID
-added VisibleForTesting annotation
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1294
This addresses bug OOZIE-1294.
https://issues.apache.org/jira/browse/OOZIE-1294
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java 1487092
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
trunk/core/src/main/java/org/apache/oozie/sla/service/SLAService.java 1487092
trunk/core/src/main/resources/oozie-default.xml 1487092
trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
Diff: https://reviews.apache.org/r/10199/diff/
Testing
-------
local test done
Thanks,
Ryota Egashira
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
> On May 28, 2013, 4:07 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java, lines 118-119
> > <https://reviews.apache.org/r/10199/diff/8/?file=297119#file297119line118>
> >
> > Should not be doing this. For every get it is going to initialize automatically.
my understanding is that this load function is only called when key doesn't exist for get call, so should be fine, but I can double-check in test case.
> On May 28, 2013, 4:07 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java, line 380
> > <https://reviews.apache.org/r/10199/diff/8/?file=297119#file297119line380>
> >
> > Annotation @VisibileForTesting. Keep it package private
good suggestion, will do that.
- Ryota
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/#review21096
-----------------------------------------------------------
On May 28, 2013, 7:52 a.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10199/
> -----------------------------------------------------------
>
> (Updated May 28, 2013, 7:52 a.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 1486210
> trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
> trunk/core/src/main/java/org/apache/oozie/sla/service/SLAService.java 1486210
> trunk/core/src/main/resources/oozie-default.xml 1486210
> trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10199/diff/
>
>
> Testing
> -------
>
> local test done
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
> On May 28, 2013, 4:07 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java, lines 118-119
> > <https://reviews.apache.org/r/10199/diff/8/?file=297119#file297119line118>
> >
> > Should not be doing this. For every get it is going to initialize automatically.
>
> Ryota Egashira wrote:
> my understanding is that this load function is only called when key doesn't exist for get call, so should be fine, but I can double-check in test case.
>
> Rohini Palaniswamy wrote:
> Looks like that is how it works. We should be good since we are using getIfPresent call. Else it would create one entry for each email address. Can you put a warning comment near getIfPresent saying "Do not change to get() call here as it will load into the cache".
hmm, even for get(), seem the same based on doc.
http://code.google.com/p/guava-libraries/wiki/CachesExplained
"The canonical way to query a LoadingCache is with the method get(K). This will either return an already cached value, or else use the cache's CacheLoader to atomically load a new value into the cache."
- Ryota
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/#review21096
-----------------------------------------------------------
On May 28, 2013, 7:52 a.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10199/
> -----------------------------------------------------------
>
> (Updated May 28, 2013, 7:52 a.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 1486210
> trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
> trunk/core/src/main/java/org/apache/oozie/sla/service/SLAService.java 1486210
> trunk/core/src/main/resources/oozie-default.xml 1486210
> trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10199/diff/
>
>
> Testing
> -------
>
> local test done
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Rohini Palaniswamy <ro...@gmail.com>.
> On May 28, 2013, 4:07 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java, lines 118-119
> > <https://reviews.apache.org/r/10199/diff/8/?file=297119#file297119line118>
> >
> > Should not be doing this. For every get it is going to initialize automatically.
>
> Ryota Egashira wrote:
> my understanding is that this load function is only called when key doesn't exist for get call, so should be fine, but I can double-check in test case.
Looks like that is how it works. We should be good since we are using getIfPresent call. Else it would create one entry for each email address. Can you put a warning comment near getIfPresent saying "Do not change to get() call here as it will load into the cache".
- Rohini
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/#review21096
-----------------------------------------------------------
On May 28, 2013, 7:52 a.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10199/
> -----------------------------------------------------------
>
> (Updated May 28, 2013, 7:52 a.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 1486210
> trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
> trunk/core/src/main/java/org/apache/oozie/sla/service/SLAService.java 1486210
> trunk/core/src/main/resources/oozie-default.xml 1486210
> trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10199/diff/
>
>
> Testing
> -------
>
> local test done
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/#review21096
-----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43837>
Should not be doing this. For every get it is going to initialize automatically.
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43835>
Annotation @VisibileForTesting. Keep it package private
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java
<https://reviews.apache.org/r/10199/#comment43836>
Do a explicit blackList.put() and remove initial load statement.
- Rohini Palaniswamy
On May 28, 2013, 7:52 a.m., Ryota Egashira wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10199/
> -----------------------------------------------------------
>
> (Updated May 28, 2013, 7:52 a.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 1486210
> trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
> trunk/core/src/main/java/org/apache/oozie/sla/service/SLAService.java 1486210
> trunk/core/src/main/resources/oozie-default.xml 1486210
> trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10199/diff/
>
>
> Testing
> -------
>
> local test done
>
>
> Thanks,
>
> Ryota Egashira
>
>
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/
-----------------------------------------------------------
(Updated May 28, 2013, 7:52 a.m.)
Review request for oozie.
Changes
-------
fixed comments.
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1294
This addresses bug OOZIE-1294.
https://issues.apache.org/jira/browse/OOZIE-1294
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java 1486210
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
trunk/core/src/main/java/org/apache/oozie/sla/service/SLAService.java 1486210
trunk/core/src/main/resources/oozie-default.xml 1486210
trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
Diff: https://reviews.apache.org/r/10199/diff/
Testing
-------
local test done
Thanks,
Ryota Egashira
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/
-----------------------------------------------------------
(Updated May 25, 2013, 8:13 a.m.)
Review request for oozie.
Changes
-------
revised rohini's comments
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1294
This addresses bug OOZIE-1294.
https://issues.apache.org/jira/browse/OOZIE-1294
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java 1486210
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
Diff: https://reviews.apache.org/r/10199/diff/
Testing (updated)
-------
local test done
Thanks,
Ryota Egashira
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/
-----------------------------------------------------------
(Updated May 23, 2013, 8:40 a.m.)
Review request for oozie.
Changes
-------
rebased patch
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1294
This addresses bug OOZIE-1294.
https://issues.apache.org/jira/browse/OOZIE-1294
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java 1485598
trunk/core/src/main/java/org/apache/oozie/sla/listener/SLAEmailEventListener.java PRE-CREATION
trunk/core/src/test/java/org/apache/oozie/sla/TestSLAEmailEventListener.java PRE-CREATION
Diff: https://reviews.apache.org/r/10199/diff/
Testing
-------
still WIP, especially error handling/test case
Thanks,
Ryota Egashira
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/
-----------------------------------------------------------
(Updated April 29, 2013, 9:28 a.m.)
Review request for oozie.
Changes
-------
change logic of blacklist to deal with transient error, such that blacklist keeps number of sendFailedException for each email, and if actual fail count >= threshold, block email.
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1294
This addresses bug OOZIE-1294.
https://issues.apache.org/jira/browse/OOZIE-1294
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java 1471127
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java PRE-CREATION
trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java 1471127
trunk/core/src/main/java/org/apache/oozie/sla/event/listener/SLAEventListener.java 1471127
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
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/
-----------------------------------------------------------
(Updated April 24, 2013, 12:29 a.m.)
Review request for oozie.
Changes
-------
fixed review comments.thanks
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1294
This addresses bug OOZIE-1294.
https://issues.apache.org/jira/browse/OOZIE-1294
Diffs (updated)
-----
trunk/core/src/main/java/org/apache/oozie/action/email/EmailActionExecutor.java 1471127
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java PRE-CREATION
trunk/core/src/main/java/org/apache/oozie/service/EventHandlerService.java 1471127
trunk/core/src/main/java/org/apache/oozie/sla/event/listener/SLAEventListener.java 1471127
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
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/#review19611
-----------------------------------------------------------
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java
<https://reviews.apache.org/r/10199/#comment40516>
yes, changed accordingly
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java
<https://reviews.apache.org/r/10199/#comment40518>
this is caught by caller method(onStartMiss/onEndMiss), which add error message, and die without sending email.
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java
<https://reviews.apache.org/r/10199/#comment40519>
thx, didn't know API supports actionID.
removed if-blocks.
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java
<https://reviews.apache.org/r/10199/#comment40520>
Transport.send / Session are thread-safe. ok to share.
also ErrorHandlerService processes event one by one sequentially.
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java
<https://reviews.apache.org/r/10199/#comment40521>
same with above.
trunk/core/src/main/java/org/apache/oozie/email/EmailSLAEventListener.java
<https://reviews.apache.org/r/10199/#comment40522>
clean up is required, since deletion happens only when the same email stored in blacklist show up, and list might grow over time. but anyway, following Rohini's suggestion, use guava Cache(https://code.google.com/p/guava-libraries/wiki/CachesExplained), which automatically takes care of cleaning up, so no problem on this.
- Ryota Egashira
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
>
>
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
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.
Changes
-------
did some change, also rebased patch.
I start to wonder if blacklist to be included or not in this version, since it adds some complexity(clean up thread, etc) and uncertainty.
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1294
This addresses bug OOZIE-1294.
https://issues.apache.org/jira/browse/OOZIE-1294
Diffs (updated)
-----
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
Re: Review Request: OOZIE-1294 SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/
-----------------------------------------------------------
(Updated March 29, 2013, 8:59 p.m.)
Review request for oozie.
Summary (updated)
-----------------
OOZIE-1294 SLA Email Notification
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/pom.xml 1462575
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
Re: Review Request: SLA Email Notification
Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10199/
-----------------------------------------------------------
(Updated March 29, 2013, 7:17 p.m.)
Review request for oozie.
Changes
-------
a bit issue in patch format (from git->svn), making open for review, will fix with other comments.
Description
-------
https://issues.apache.org/jira/browse/OOZIE-1294
This addresses bug OOZIE-1294.
https://issues.apache.org/jira/browse/OOZIE-1294
Diffs (updated)
-----
trunk/core/pom.xml 1462575
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