You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Peeyush Bishnoi <bp...@yahoo.co.in> on 2015/09/03 20:08:37 UTC
Review Request 38105: FALCON-1425: Provide Email based notification
plugin to send notification when Falcon instance completes.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/
-----------------------------------------------------------
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 3d558fc
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 3aaf304
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 52b558d
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
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> On Sept. 11, 2015, 2:06 a.m., Venkat Ranganathan wrote:
> > Thanks for working on this. Also can you more unit tests were applicable (for example, email address validation, malformed emain notification list (trailing comma ) etc
>
> Peeyush Bishnoi wrote:
> Thanks for the review. I have increased the coverage of unit tests for EmailNotification.
Thanks! Venkat for the review. I have incorporated your comments in the patch.
> On Sept. 11, 2015, 2:06 a.m., Venkat Ranganathan wrote:
> > client/src/main/resources/feed-0.1.xsd, line 304
> > <https://reviews.apache.org/r/38105/diff/4/?file=1065396#file1065396line304>
> >
> > Initially we were discussing an option to send notification for all retries vs sending after exhaustion of all retries (which will be the default). Don't we need some attribute to distringuish that?
>
> Peeyush Bishnoi wrote:
> Venkat, I am of the opinion that if user has defined the retry tag in Entity, then we should send notification after exhaustion of all retries. If user has not defined the retry in Entity, we will send after Instance failure only(one attempt). We will document this so that user should be aware of properly. So having the retry tag in Entity will help to distinguish itself. To handle this issue, I am working on FALCON-1431/FALCON-1433. Thoughts please
>
> Venkat Ranganathan wrote:
> If I defined retries, then I can expect an email if ever attempt fails or only if all the retries are exhausted right? Isn't that what we thought we will provide in future with only one notification after exhaustion of all retries being the default?
>
> Peeyush Bishnoi wrote:
> 1. If retry is defined along with notification and Falcon instance fail
> <process name ....>
> ...
> ...
> <retry policy="periodic" delay="minutes(2)" attempts="3"/>
> <notification type="email" to="falcon@c6001.example.com"/>
> </process>
>
> then failure email will be sent only if all the retries attempt (3) are exhausted for Falcon instance.
>
> 2. If retry is not defined along with notification and Falcon instance fail, then failure email will be sent even if attempt fails.
>
> Please let me know if this approach is fine.
>
>
> Presently, there are issues with failure retry which I am working in separate issue FALCON-1431/FALCON-1433 once that get fix, step 1 will work.
As per discussion we had, I have added a new "limit" property in notification element of Feed/process xsd with value: "attempt" and "final". We will put details around this property once FALCON-1433 get fix.
- Peeyush
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review98397
-----------------------------------------------------------
On Sept. 11, 2015, 3:09 p.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2015, 3:09 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 3d558fc
> 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 3aaf304
> oozie/src/test/resources/config/process/process-notification.xml PRE-CREATION
> oozie/src/test/resources/feed/feed-notification.xml PRE-CREATION
> prism/pom.xml 52b558d
> 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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> On Sept. 11, 2015, 2:06 a.m., Venkat Ranganathan wrote:
> > client/src/main/resources/feed-0.1.xsd, line 304
> > <https://reviews.apache.org/r/38105/diff/4/?file=1065396#file1065396line304>
> >
> > Initially we were discussing an option to send notification for all retries vs sending after exhaustion of all retries (which will be the default). Don't we need some attribute to distringuish that?
>
> Peeyush Bishnoi wrote:
> Venkat, I am of the opinion that if user has defined the retry tag in Entity, then we should send notification after exhaustion of all retries. If user has not defined the retry in Entity, we will send after Instance failure only(one attempt). We will document this so that user should be aware of properly. So having the retry tag in Entity will help to distinguish itself. To handle this issue, I am working on FALCON-1431/FALCON-1433. Thoughts please
>
> Venkat Ranganathan wrote:
> If I defined retries, then I can expect an email if ever attempt fails or only if all the retries are exhausted right? Isn't that what we thought we will provide in future with only one notification after exhaustion of all retries being the default?
1. If retry is defined along with notification and Falcon instance fail
<process name ....>
...
...
<retry policy="periodic" delay="minutes(2)" attempts="3"/>
<notification type="email" to="falcon@c6001.example.com"/>
</process>
then failure email will be sent only if all the retries attempt (3) are exhausted for Falcon instance.
2. If retry is not defined along with notification and Falcon instance fail, then failure email will be sent even if attempt fails.
Please let me know if this approach is fine.
Presently, there are issues with failure retry which I am working in separate issue FALCON-1431/FALCON-1433 once that get fix, step 1 will work.
- Peeyush
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review98397
-----------------------------------------------------------
On Sept. 11, 2015, 3:09 p.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2015, 3:09 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 3d558fc
> 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 3aaf304
> oozie/src/test/resources/config/process/process-notification.xml PRE-CREATION
> oozie/src/test/resources/feed/feed-notification.xml PRE-CREATION
> prism/pom.xml 52b558d
> 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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Venkat Ranganathan <n....@live.com>.
> On Sept. 10, 2015, 7:06 p.m., Venkat Ranganathan wrote:
> > client/src/main/resources/feed-0.1.xsd, line 304
> > <https://reviews.apache.org/r/38105/diff/4/?file=1065396#file1065396line304>
> >
> > Initially we were discussing an option to send notification for all retries vs sending after exhaustion of all retries (which will be the default). Don't we need some attribute to distringuish that?
>
> Peeyush Bishnoi wrote:
> Venkat, I am of the opinion that if user has defined the retry tag in Entity, then we should send notification after exhaustion of all retries. If user has not defined the retry in Entity, we will send after Instance failure only(one attempt). We will document this so that user should be aware of properly. So having the retry tag in Entity will help to distinguish itself. To handle this issue, I am working on FALCON-1431/FALCON-1433. Thoughts please
If I defined retries, then I can expect an email if ever attempt fails or only if all the retries are exhausted right? Isn't that what we thought we will provide in future with only one notification after exhaustion of all retries being the default?
> On Sept. 10, 2015, 7:06 p.m., Venkat Ranganathan wrote:
> > client/src/main/resources/feed-0.1.xsd, line 117
> > <https://reviews.apache.org/r/38105/diff/4/?file=1065396#file1065396line117>
> >
> > We are adding new eleements to an XSD without modifying the version. It does not look like a good practice. As I mentioned during the lifecycle discussions, we should have a version managmenet approach of XSDs.
> >
> > Something like:
> >
> > Major version change - incompatible changes
> > Minor version change - compatible changes
> > (We don't need to have more than one level of schema versioning unless it is felt otherwise by the community)
>
> Peeyush Bishnoi wrote:
> Venkat, I agree that XSD version should be changed when new element is added. Even I have tried to change the xsd version to 0.2 but looks like version mapping internally is tightly bound as I am seeing lot of issues and build is getting failed. I have created separate issue FALCON-1444 for bumping the XSD version.
OK, good. I think I raised it in one of the discussions about Lifecycle bringing in changes. And there is this datasource enttity related changes and we also have changes that will come in with authorization changes. Let me start a separte thread to discuss this
- Venkat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review98397
-----------------------------------------------------------
On Sept. 11, 2015, 8:09 a.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2015, 8:09 a.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 3d558fc
> 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 3aaf304
> oozie/src/test/resources/config/process/process-notification.xml PRE-CREATION
> oozie/src/test/resources/feed/feed-notification.xml PRE-CREATION
> prism/pom.xml 52b558d
> 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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> On Sept. 11, 2015, 2:06 a.m., Venkat Ranganathan wrote:
> > Thanks for working on this. Also can you more unit tests were applicable (for example, email address validation, malformed emain notification list (trailing comma ) etc
Thanks for the review. I have increased the coverage of unit tests for EmailNotification.
> On Sept. 11, 2015, 2:06 a.m., Venkat Ranganathan wrote:
> > client/src/main/resources/feed-0.1.xsd, line 304
> > <https://reviews.apache.org/r/38105/diff/4/?file=1065396#file1065396line304>
> >
> > Initially we were discussing an option to send notification for all retries vs sending after exhaustion of all retries (which will be the default). Don't we need some attribute to distringuish that?
Venkat, I am of the opinion that if user has defined the retry tag in Entity, then we should send notification after exhaustion of all retries. If user has not defined the retry in Entity, we will send after Instance failure only(one attempt). We will document this so that user should be aware of properly. So having the retry tag in Entity will help to distinguish itself. To handle this issue, I am working on FALCON-1431/FALCON-1433. Thoughts please
> On Sept. 11, 2015, 2:06 a.m., Venkat Ranganathan wrote:
> > client/src/main/resources/feed-0.1.xsd, line 117
> > <https://reviews.apache.org/r/38105/diff/4/?file=1065396#file1065396line117>
> >
> > We are adding new eleements to an XSD without modifying the version. It does not look like a good practice. As I mentioned during the lifecycle discussions, we should have a version managmenet approach of XSDs.
> >
> > Something like:
> >
> > Major version change - incompatible changes
> > Minor version change - compatible changes
> > (We don't need to have more than one level of schema versioning unless it is felt otherwise by the community)
Venkat, I agree that XSD version should be changed when new element is added. Even I have tried to change the xsd version to 0.2 but looks like version mapping internally is tightly bound as I am seeing lot of issues and build is getting failed. I have created separate issue FALCON-1444 for bumping the XSD version.
- Peeyush
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review98397
-----------------------------------------------------------
On Sept. 8, 2015, 6:50 p.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2015, 6:50 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.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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review98397
-----------------------------------------------------------
Thanks for working on this. Also can you more unit tests were applicable (for example, email address validation, malformed emain notification list (trailing comma ) etc
client/src/main/resources/feed-0.1.xsd (line 117)
<https://reviews.apache.org/r/38105/#comment154868>
We are adding new eleements to an XSD without modifying the version. It does not look like a good practice. As I mentioned during the lifecycle discussions, we should have a version managmenet approach of XSDs.
Something like:
Major version change - incompatible changes
Minor version change - compatible changes
(We don't need to have more than one level of schema versioning unless it is felt otherwise by the community)
client/src/main/resources/feed-0.1.xsd (line 304)
<https://reviews.apache.org/r/38105/#comment154869>
Initially we were discussing an option to send notification for all retries vs sending after exhaustion of all retries (which will be the default). Don't we need some attribute to distringuish that?
- Venkat Ranganathan
On Sept. 8, 2015, 11:50 a.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2015, 11:50 a.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.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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> On Sept. 10, 2015, 6:22 a.m., Pallavi Rao wrote:
> > Apart from a request for an additional UT, the code looks good me.
Thanks Pallavi for the review.
> On Sept. 10, 2015, 6:22 a.m., Pallavi Rao wrote:
> > prism/src/main/java/org/apache/falcon/util/NotificationUtil.java, line 48
> > <https://reviews.apache.org/r/38105/diff/4/?file=1065413#file1065413line48>
> >
> > Can add a UT for this one too. Should be straight forward.
Increased the coverage of unit tests for EmailNotification.
- Peeyush
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review98362
-----------------------------------------------------------
On Sept. 8, 2015, 6:50 p.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2015, 6:50 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.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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Pallavi Rao <pa...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review98362
-----------------------------------------------------------
Ship it!
Apart from a request for an additional UT, the code looks good me.
prism/src/main/java/org/apache/falcon/util/NotificationUtil.java (line 48)
<https://reviews.apache.org/r/38105/#comment154787>
Can add a UT for this one too. Should be straight forward.
- Pallavi Rao
On Sept. 8, 2015, 6:50 p.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2015, 6:50 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.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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > This is a very useful feature. It will be very useful to have a one page documentation in the docs on how to use it and it's behavior (e.g. email coming only after all retries or otherwise)
Thanks! Ajay for reviewing the patch. I have incorporated your comments in the patch.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > client/src/main/java/org/apache/falcon/entity/v0/EntityNotification.java, line 29
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068323#file1068323line29>
> >
> > It will be more useful to have marshalled xml in toString, since it is a type for xsd and is used in xmls.
I have checked other xsd type toString and found that even they are not marshalled xml. Will create separate issue to collectively perform this for all xsd type.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > client/src/main/java/org/apache/falcon/entity/v0/EntityNotification.java, line 26
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068323#file1068323line26>
> >
> > What is type? What is it used for?
Here type is related to Notification Type from xsd (<notification type="email" to="falcon@c6001.example.com"/>). EntityNotification is the base class for Feed Notification (org.apache.falcon.entity.v0.feed.Notification) and Process Notification (org.apache.falcon.entity.v0.process.Notification). So getType from EntityNotification should match to JAXB generated Notification.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > prism/src/main/java/org/apache/falcon/util/NotificationUtil.java, line 44
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068341#file1068341line44>
> >
> > Just an observation, this pattern will allow some invalid domains.
> >
> > e.g. ajay.yadav@gmail.com.1
> >
> >
> > Although a perfect validation is not required as this is still a runtime check and mail will fail in any case. Following might be a better regex.
> >
> > "^[_A-Za-z0-9-\+]+(\.[_A-Za-z0-9-]+)*@"
> > + "[A-Za-z0-9-]+(\.[A-Za-z0-9]+)*(\.[A-Za-z]{2,})$"
Earlier I have used the similar regex which you have suggested for email pattern. But I have relaxed this so that other required email pattern like falcon@localhost etc. should match.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > prism/src/main/java/org/apache/falcon/plugin/EmailNotification.java, line 110
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068338#file1068338line110>
> >
> > This should be under the "if - else if" to avoid sending email for alerts other than "wf-instance-succeeded" and "wf-instance-failed"
Check for this is already available in class EmailNotificationPlugin. Please check.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > prism/src/main/java/org/apache/falcon/plugin/EmailNotification.java, line 45
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068338#file1068338line45>
> >
> > There are 2 classes:
> >
> > 1) EmailNotification implements NotificationPlugin
> > 2) EmailNotificationPlugin implements MonitoringPlugin
> >
> > They are both confusing in terms of name. There should be just one class which implements both monitoring plugin and NotificationPlugin and called EmailNotificationPlugin.
To send the notification events through email we have to extend the monitoring framework by implementing org.apache.falcon.plugin.MonitoringPlugin interface. Due to this EmailNotificationPlugin require MonitoringPlugin(similar to DefaultMonitoringPlugin). But for actual notification implementation we require to implement the NotificationPlugin interface for sending the notification events and in future it should be used by any other notification mechanism like Kafka, HTTP etc.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > oozie/src/test/java/org/apache/falcon/oozie/process/OozieProcessWorkflowBuilderTest.java, line 222
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068334#file1068334line222>
> >
> > Should be part of ProcessEntityParserTest.
Moved the testcase to class ProcessEntityParserTest.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > oozie/src/test/java/org/apache/falcon/oozie/feed/OozieFeedWorkflowBuilderTest.java, line 286
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068333#file1068333line286>
> >
> > this test should be part of FeedEntityParserTest. It has nothing to do with workflowbuilding or oozie.
Moved the testcase to class FeedEntityParserTest.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > metrics/src/main/java/org/apache/falcon/plugin/NotificationPlugin.java, line 27
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068330#file1068330line27>
> >
> > This interface has no characterstics of a plugin unlike others e.g. MonitoringPlugin. It should be named as just Notification.
Notification class has already been used by notification element from feed/process xsd, so can't reuse the name again. Also if going forward, other notification (like Kafka/HTTP) implementation required to send notification for captured events, this class can be implemented.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > metrics/src/main/java/org/apache/falcon/plugin/NotificationPlugin.java, line 25
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068330#file1068330line25>
> >
> > There is no "Notification concrete class". Perhaps you meant EmailNotification.java.
> >
> > In any case it is not a good documentation for this interface. A short description of what is the contract that this interface provides will be useful.
Done.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > client/src/main/resources/process-0.1.xsd, line 412
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068326#file1068326line412>
> >
> > same as feed.xsd
done.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > client/src/main/resources/feed-0.1.xsd, line 304
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068324#file1068324line304>
> >
> > It will be good to document that the to list can contain multiple email addresseses separated by comma.
Done.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > client/src/main/resources/feed-0.1.xsd, line 303
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068324#file1068324line303>
> >
> > type should be of xs:enumeration instead of xs:string because any random string won't be accepted by falcon.
Done. Currently only type ="email" will be accepted. Any other type entered will not be accepted by xsd.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > client/src/main/java/org/apache/falcon/entity/v0/EntityNotification.java, line 23
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068323#file1068323line23>
> >
> > nit: this documentation can use some more explanation.
> > What is meant by notification list - "recepients list" or "type of notifications"?
Done.
> On Sept. 14, 2015, 1:48 p.m., Ajay Yadava wrote:
> > metrics/src/main/java/org/apache/falcon/util/EmailNotificationArgs.java, line 24
> > <https://reviews.apache.org/r/38105/diff/5/?file=1068331#file1068331line24>
> >
> > These are not arguments for Email Notification, these are just constants for SMTP properties. Should be renamed accordingly.
Renamed to EmailNotificationProps. Done.
- Peeyush
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review98677
-----------------------------------------------------------
On Sept. 11, 2015, 3:09 p.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2015, 3:09 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 3d558fc
> 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 3aaf304
> oozie/src/test/resources/config/process/process-notification.xml PRE-CREATION
> oozie/src/test/resources/feed/feed-notification.xml PRE-CREATION
> prism/pom.xml 52b558d
> 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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review98677
-----------------------------------------------------------
This is a very useful feature. It will be very useful to have a one page documentation in the docs on how to use it and it's behavior (e.g. email coming only after all retries or otherwise)
client/src/main/java/org/apache/falcon/entity/v0/EntityNotification.java (line 23)
<https://reviews.apache.org/r/38105/#comment155436>
nit: this documentation can use some more explanation.
What is meant by notification list - "recepients list" or "type of notifications"?
client/src/main/java/org/apache/falcon/entity/v0/EntityNotification.java (line 26)
<https://reviews.apache.org/r/38105/#comment155437>
What is type? What is it used for?
client/src/main/java/org/apache/falcon/entity/v0/EntityNotification.java (line 29)
<https://reviews.apache.org/r/38105/#comment155438>
It will be more useful to have marshalled xml in toString, since it is a type for xsd and is used in xmls.
client/src/main/resources/feed-0.1.xsd (line 303)
<https://reviews.apache.org/r/38105/#comment155439>
type should be of xs:enumeration instead of xs:string because any random string won't be accepted by falcon.
client/src/main/resources/feed-0.1.xsd (line 304)
<https://reviews.apache.org/r/38105/#comment155447>
It will be good to document that the to list can contain multiple email addresseses separated by comma.
client/src/main/resources/process-0.1.xsd (line 412)
<https://reviews.apache.org/r/38105/#comment155440>
same as feed.xsd
metrics/src/main/java/org/apache/falcon/plugin/NotificationPlugin.java (line 25)
<https://reviews.apache.org/r/38105/#comment155445>
There is no "Notification concrete class". Perhaps you meant EmailNotification.java.
In any case it is not a good documentation for this interface. A short description of what is the contract that this interface provides will be useful.
metrics/src/main/java/org/apache/falcon/plugin/NotificationPlugin.java (line 27)
<https://reviews.apache.org/r/38105/#comment155444>
This interface has no characterstics of a plugin unlike others e.g. MonitoringPlugin. It should be named as just Notification.
metrics/src/main/java/org/apache/falcon/util/EmailNotificationArgs.java (line 24)
<https://reviews.apache.org/r/38105/#comment155446>
These are not arguments for Email Notification, these are just constants for SMTP properties. Should be renamed accordingly.
oozie/src/test/java/org/apache/falcon/oozie/feed/OozieFeedWorkflowBuilderTest.java (line 286)
<https://reviews.apache.org/r/38105/#comment155441>
this test should be part of FeedEntityParserTest. It has nothing to do with workflowbuilding or oozie.
oozie/src/test/java/org/apache/falcon/oozie/process/OozieProcessWorkflowBuilderTest.java (line 222)
<https://reviews.apache.org/r/38105/#comment155442>
Should be part of ProcessEntityParserTest.
prism/src/main/java/org/apache/falcon/plugin/EmailNotification.java (line 45)
<https://reviews.apache.org/r/38105/#comment155443>
There are 2 classes:
1) EmailNotification implements NotificationPlugin
2) EmailNotificationPlugin implements MonitoringPlugin
They are both confusing in terms of name. There should be just one class which implements both monitoring plugin and NotificationPlugin and called EmailNotificationPlugin.
prism/src/main/java/org/apache/falcon/plugin/EmailNotification.java (line 110)
<https://reviews.apache.org/r/38105/#comment155455>
This should be under the "if - else if" to avoid sending email for alerts other than "wf-instance-succeeded" and "wf-instance-failed"
prism/src/main/java/org/apache/falcon/util/NotificationUtil.java (line 44)
<https://reviews.apache.org/r/38105/#comment155454>
Just an observation, this pattern will allow some invalid domains.
e.g. ajay.yadav@gmail.com.1
Although a perfect validation is not required as this is still a runtime check and mail will fail in any case. Following might be a better regex.
"^[_A-Za-z0-9-\+]+(\.[_A-Za-z0-9-]+)*@"
+ "[A-Za-z0-9-]+(\.[A-Za-z0-9]+)*(\.[A-Za-z]{2,})$"
- Ajay Yadava
On Sept. 11, 2015, 3:09 p.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2015, 3:09 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 3d558fc
> 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 3aaf304
> oozie/src/test/resources/config/process/process-notification.xml PRE-CREATION
> oozie/src/test/resources/feed/feed-notification.xml PRE-CREATION
> prism/pom.xml 52b558d
> 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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review99341
-----------------------------------------------------------
Ship it!
Ship It!
- Ajay Yadava
On Sept. 16, 2015, 7:05 p.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 16, 2015, 7:05 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 2f05b1f
> common/src/main/resources/startup.properties 0593b96
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 754fb06
> common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java 6612b74
> common/src/test/resources/config/feed/feed-0.1.xml 6448803
> common/src/test/resources/config/process/process-0.1.xml 2659903
> metrics/pom.xml 748fb97
> metrics/src/main/java/org/apache/falcon/plugin/NotificationPlugin.java PRE-CREATION
> metrics/src/main/java/org/apache/falcon/util/EmailNotificationProps.java PRE-CREATION
> metrics/src/main/java/org/apache/falcon/util/NotificationType.java 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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.java PRE-CREATION
> src/conf/startup.properties ca55689
>
> 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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/
-----------------------------------------------------------
(Updated Sept. 16, 2015, 7:05 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 (updated)
-----
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 2f05b1f
common/src/main/resources/startup.properties 0593b96
common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 754fb06
common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java 6612b74
common/src/test/resources/config/feed/feed-0.1.xml 6448803
common/src/test/resources/config/process/process-0.1.xml 2659903
metrics/pom.xml 748fb97
metrics/src/main/java/org/apache/falcon/plugin/NotificationPlugin.java PRE-CREATION
metrics/src/main/java/org/apache/falcon/util/EmailNotificationProps.java PRE-CREATION
metrics/src/main/java/org/apache/falcon/util/NotificationType.java 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
prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.java PRE-CREATION
src/conf/startup.properties ca55689
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.
Unit test has been added to test Email Notification.
Thanks,
Peeyush Bishnoi
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> On Sept. 16, 2015, 2:57 p.m., Venkat Ranganathan wrote:
> > client/src/main/resources/feed-0.1.xsd, line 322
> > <https://reviews.apache.org/r/38105/diff/6/?file=1073912#file1073912line322>
> >
> > Instead of limit, do you think level is a better terminology? Or may be trigger, but that sounds too heavy.
> > For example
> > level="attempt" or level = "instance"
done.
> On Sept. 16, 2015, 2:57 p.m., Venkat Ranganathan wrote:
> > client/src/main/resources/feed-0.1.xsd, line 326
> > <https://reviews.apache.org/r/38105/diff/6/?file=1073912#file1073912line326>
> >
> > Would suggest instance instead of final
done.
- Peeyush
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review99222
-----------------------------------------------------------
On Sept. 15, 2015, 3:11 p.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2015, 3:11 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 2f05b1f
> common/src/main/resources/startup.properties 0593b96
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 754fb06
> common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java 6612b74
> common/src/test/resources/config/feed/feed-0.1.xml 6448803
> common/src/test/resources/config/process/process-0.1.xml 2659903
> metrics/pom.xml 748fb97
> metrics/src/main/java/org/apache/falcon/plugin/NotificationPlugin.java PRE-CREATION
> metrics/src/main/java/org/apache/falcon/util/EmailNotificationProps.java PRE-CREATION
> metrics/src/main/java/org/apache/falcon/util/NotificationType.java 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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.java PRE-CREATION
> src/conf/startup.properties ca55689
>
> 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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review99222
-----------------------------------------------------------
Looks good. A couple of suggestions
client/src/main/resources/feed-0.1.xsd (line 322)
<https://reviews.apache.org/r/38105/#comment156157>
Instead of limit, do you think level is a better terminology? Or may be trigger, but that sounds too heavy.
For example
level="attempt" or level = "instance"
client/src/main/resources/feed-0.1.xsd (line 326)
<https://reviews.apache.org/r/38105/#comment156158>
Would suggest instance instead of final
- Venkat Ranganathan
On Sept. 15, 2015, 8:11 a.m., Peeyush Bishnoi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38105/
> -----------------------------------------------------------
>
> (Updated Sept. 15, 2015, 8:11 a.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 2f05b1f
> common/src/main/resources/startup.properties 0593b96
> common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 754fb06
> common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java 6612b74
> common/src/test/resources/config/feed/feed-0.1.xml 6448803
> common/src/test/resources/config/process/process-0.1.xml 2659903
> metrics/pom.xml 748fb97
> metrics/src/main/java/org/apache/falcon/plugin/NotificationPlugin.java PRE-CREATION
> metrics/src/main/java/org/apache/falcon/util/EmailNotificationProps.java PRE-CREATION
> metrics/src/main/java/org/apache/falcon/util/NotificationType.java 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
> prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.java PRE-CREATION
> src/conf/startup.properties ca55689
>
> 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.
> Unit test has been added to test Email Notification.
>
>
> Thanks,
>
> Peeyush Bishnoi
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/
-----------------------------------------------------------
(Updated Sept. 15, 2015, 3:11 p.m.)
Review request for Falcon.
Changes
-------
Updated patch after incorporating the review comments.
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 (updated)
-----
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 2f05b1f
common/src/main/resources/startup.properties 0593b96
common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 754fb06
common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java 6612b74
common/src/test/resources/config/feed/feed-0.1.xml 6448803
common/src/test/resources/config/process/process-0.1.xml 2659903
metrics/pom.xml 748fb97
metrics/src/main/java/org/apache/falcon/plugin/NotificationPlugin.java PRE-CREATION
metrics/src/main/java/org/apache/falcon/util/EmailNotificationProps.java PRE-CREATION
metrics/src/main/java/org/apache/falcon/util/NotificationType.java 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
prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.java PRE-CREATION
src/conf/startup.properties ca55689
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.
Unit test has been added to test Email Notification.
Thanks,
Peeyush Bishnoi
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/
-----------------------------------------------------------
(Updated Sept. 11, 2015, 3:09 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 (updated)
-----
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 3d558fc
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 3aaf304
oozie/src/test/resources/config/process/process-notification.xml PRE-CREATION
oozie/src/test/resources/feed/feed-notification.xml PRE-CREATION
prism/pom.xml 52b558d
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
prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
Unit test has been added to test Email Notification.
Thanks,
Peeyush Bishnoi
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/
-----------------------------------------------------------
(Updated Sept. 8, 2015, 6:50 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 (updated)
-----
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.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
prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.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.
Unit test has been added to test Email Notification.
Thanks,
Peeyush Bishnoi
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/
-----------------------------------------------------------
(Updated Sept. 8, 2015, 7:34 a.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 (updated)
-----
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.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
prism/src/test/java/org/apache/falcon/plugin/EmailNotificationTest.java PRE-CREATION
src/conf/startup.properties 9925373
Diff: https://reviews.apache.org/r/38105/diff/
Testing (updated)
-------
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.
Unit test has been added to test Email Notification.
Thanks,
Peeyush Bishnoi
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> On Sept. 7, 2015, 4:51 a.m., Pallavi Rao wrote:
> >
Thanks for reviewing the patch. I have uploaded new patch by incorporating the comments.
- 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
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
> 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
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Pallavi Rao <pa...@inmobi.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38105/#review97889
-----------------------------------------------------------
client/src/main/resources/feed-0.1.xsd (line 117)
<https://reviews.apache.org/r/38105/#comment153963>
Add doc here as you have done in process xsd.
client/src/main/resources/feed-0.1.xsd (line 297)
<https://reviews.apache.org/r/38105/#comment153960>
I see that you have kept the notification generic (it can be http tomorrow). In that case, should we call this "destination" instead?
client/src/main/resources/process-0.1.xsd (line 176)
<https://reviews.apache.org/r/38105/#comment153961>
Nit : Can we reword this? Currently supported notification types are [email]. Specify the email destination address to receive email notifications.
metrics/pom.xml (line 66)
<https://reviews.apache.org/r/38105/#comment153964>
Why 1.4 when 1.4.7 is the latest?
metrics/src/main/java/org/apache/falcon/util/EmailNotificationArgs.java (line 28)
<https://reviews.apache.org/r/38105/#comment153965>
Nit : Add to startup.properties as a commented line, so users know the arg to set.
oozie/src/test/java/org/apache/falcon/oozie/feed/OozieFeedWorkflowBuilderTest.java (line 287)
<https://reviews.apache.org/r/38105/#comment153966>
Nit : Rename the method to reflect what it is doing.. "testEmailNotification" should suffice.
prism/src/main/java/org/apache/falcon/plugin/EmailNotification.java (line 43)
<https://reviews.apache.org/r/38105/#comment153967>
This class and other classes following this are completely missing UTs. Can we add some UTs, please?
- Pallavi Rao
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
>
>
Re: Review Request 38105: FALCON-1425: Provide Email based
notification
plugin to send notification when Falcon instance completes.
Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
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 (updated)
-----
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