Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Srikanth Sundarrajan]

> On Jan. 18, 2016, 5:31 a.m., Rohini Palaniswamy wrote:
> > client/src/main/resources/callback-action-0.1.xsd, lines 28-31
> > 
> >
> > Would suggest two additions - type and configuration.
> > 
> > url
> > type
> > method
> > configuration
> > arg
> > capture-output
> > 
> > - type can be http or jms. This can be used to abstract the diffent 
> > options supported in different classes and instantiated via factory making 
> > it more clean and extensible. With type, method can then just be GET, PUT 
> > and POST. 
> > - Use configuration instead of arg if it is property with name and 
> > value. 
> > - Can still keep arg to pass arguments like in pig/hive/spark actions 
> > and use Options parser to parse the arguments if needed.
> 
> Jaydeep Vishwakarma wrote:
> In future we can have some other way of sending notification.  GET, PUT 
> and POST cannot be generic,  As these terminology are only meaningful for 
> http calls.
> It does not belongs to configuration category according to it 
> characterstics. 
> I always felt to have more cleaner way for  pig/hive/spark action as 
> well. The current format making it more cleaner. 
> Please let me know if you have diffrent thought.
> 
> Rohini Palaniswamy wrote:
> > In future we can have some other way of sending notification.  GET, PUT 
> and POST cannot be generic,  As these terminology are only meaningful for 
> http calls.
>   
>   Yes. For that very reason, you definitely need type. When type is 
> defined as HTTP, method values will be GET, PUT and POST. That is more clean 
> instead of adding multiple different kinds of methods like HTTP_GET, 
> HTTP_PUT, QUEUE_PUBLISH and some XXX in future without any relation between 
> them.
>   
> > I always felt to have more cleaner way for  pig/hive/spark action as 
> well.
>   
>   Pig/hive/spark/distcp are clean. They have configuration which means 
> actual configuration used while running and arg to mean the command line 
> arguments they take. It is easy for a commandline user to relate to as there 
> is direct correlation. I agree with you configuration does not make sense in 
> this case as it is actual data that you are passing there and not 
> configuration. But arg does not make sense either as it is not arguments. In 
> traditional sense, arguments should be based on commandline options like 
> (-key1 val1 -key2 val2) or atleast positional (first argument is is value for 
> key1, second argument is value for key2, etc). You can go probably go with 
> params instead.
> 
> Robert Kanter wrote:
> This is a good point.  I agree with Rohini that we should revisit the 
> user-facing aspects of the action and make sure it's right and generic 
> enough.  (For example, users still find it confusing to enter the RM address 
> in the  field when using Hadoop 2)

My 2 cents on inclusion of type & relating to customization of headers/payload 
(covered in a subsequent comment):

The feature request was to provide a simple callback /notification mechanism 
with the ability to customize the callback with properties from the workflow / 
coordinator or both (as covered in the initial exchanges in the jira). Also 
another important ask was to allow for this callback to be lightweight and be 
performed from within the oozie server without adopting the general scheme of 
executing action through a MR-launcher. This wasn't intended to be a general 
http client, which might then require support for overriding headers or to 
provide a stream entity in the post/put body (doing so from within the oozie 
server might be a bad idea, as the payload can be fairly unbounded). The way to 
look at the callback action definition is that there is a flattened method 
which descibes the implementation of the callback and the arguments which were 
meant to a general purpose KV map to be shared. How the KV map need to be 
marshalled is left to the implementation of the callback. For ex. An HTTP
 _GET would send them as a query param, while JMS_PUBLISH would send them as a 
MapMessage. Or in other words, the contract is simply, "how to notify/callback" 
- method and the "what parameters to send during notification" - args/config, 
thus keeping it both simple and very pointed towards the callback use case. 

Since the callback parameters are KV pairs, use of configuration instead of 
args may be more appropriate.


- Srikanth


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


On Jan. 18, 2016, 1:28 p.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Robert Kanter]

> On Jan. 18, 2016, 5:31 a.m., Rohini Palaniswamy wrote:
> > client/src/main/resources/callback-action-0.1.xsd, lines 28-31
> > 
> >
> > Would suggest two additions - type and configuration.
> > 
> > url
> > type
> > method
> > configuration
> > arg
> > capture-output
> > 
> > - type can be http or jms. This can be used to abstract the diffent 
> > options supported in different classes and instantiated via factory making 
> > it more clean and extensible. With type, method can then just be GET, PUT 
> > and POST. 
> > - Use configuration instead of arg if it is property with name and 
> > value. 
> > - Can still keep arg to pass arguments like in pig/hive/spark actions 
> > and use Options parser to parse the arguments if needed.
> 
> Jaydeep Vishwakarma wrote:
> In future we can have some other way of sending notification.  GET, PUT 
> and POST cannot be generic,  As these terminology are only meaningful for 
> http calls.
> It does not belongs to configuration category according to it 
> characterstics. 
> I always felt to have more cleaner way for  pig/hive/spark action as 
> well. The current format making it more cleaner. 
> Please let me know if you have diffrent thought.
> 
> Rohini Palaniswamy wrote:
> > In future we can have some other way of sending notification.  GET, PUT 
> and POST cannot be generic,  As these terminology are only meaningful for 
> http calls.
>   
>   Yes. For that very reason, you definitely need type. When type is 
> defined as HTTP, method values will be GET, PUT and POST. That is more clean 
> instead of adding multiple different kinds of methods like HTTP_GET, 
> HTTP_PUT, QUEUE_PUBLISH and some XXX in future without any relation between 
> them.
>   
> > I always felt to have more cleaner way for  pig/hive/spark action as 
> well.
>   
>   Pig/hive/spark/distcp are clean. They have configuration which means 
> actual configuration used while running and arg to mean the command line 
> arguments they take. It is easy for a commandline user to relate to as there 
> is direct correlation. I agree with you configuration does not make sense in 
> this case as it is actual data that you are passing there and not 
> configuration. But arg does not make sense either as it is not arguments. In 
> traditional sense, arguments should be based on commandline options like 
> (-key1 val1 -key2 val2) or atleast positional (first argument is is value for 
> key1, second argument is value for key2, etc). You can go probably go with 
> params instead.

This is a good point.  I agree with Rohini that we should revisit the 
user-facing aspects of the action and make sure it's right and generic enough.  
(For example, users still find it confusing to enter the RM address in the 
 field when using Hadoop 2)


- Robert


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


On Jan. 18, 2016, 1:28 p.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated Jan. 18, 2016, 1:28 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/CallBack.java 
> PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/HTTPCallBack.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSCallBack.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/CallbackActionService.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 0a7e250 
>   
> core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
>  PRE-CREATION 
>   docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
>   docs/src/site/twiki/index.twiki 8591530 
> 
> Diff: https://reviews.apache.org/r/36123/diff/
> 
> 
> Testing
> ---
> 
> Done, 
> Will add more 

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Rohini Palaniswamy]

> On Jan. 18, 2016, 5:31 a.m., Rohini Palaniswamy wrote:
> > docs/src/site/twiki/DG_CallbackActionExtension.twiki, lines 64-66
> > 
> >
> > Currently it only seems to be contents of body of MapMessage. In 
> > future, if support for Message properties has to be added how is that 
> > supposed to be done? Or if different type of Message - says TextMessage has 
> > to be supported.
> > 
> > In case of http get, it seems to be query params and with post it seems 
> > to be form params (application/x-www-form-urlencoded). What if 
> > application/octet-stream needs to be supported in the future? 
> > 
> > I think we need to define and document all this clearly and also make 
> > sure it is not difficult to enhance in future. As I see it, the current 
> > form of definition makes it hard to support additional features of HTTP or 
> > JMS in future without hacking around how arguments are defined as we are 
> > assuming now by default there is only one way of doing it.
> 
> Jaydeep Vishwakarma wrote:
> I agree,  I will detail out all the details in doc. 
> I have created a jira OOZIE-2403 for custom method. I will take care the 
> future enchancement part there.
> 
> Rohini Palaniswamy wrote:
> I am not asking about adding those future enhancements. The current xsd 
> and argument/data passing interface is not clean enough to extend in future. 
> That needs to be fixed here taking future enhancements into account. Even 
> current usage is not even clear from documentation. I had to look into code 
> to understand what kind of data and parameters are constructed for all 
> methods and what needs to be actually passed in the arg section by the user 
> for them to be used.

One more thing I missed mentioning is HTTP headers (to specify content type, 
etc). Basically what I am asking is to go back and revisit all options/features 
associated with HTTP and JMS and come up with a generic way for user to easily 
and intuitively represent them in xml without confusion. Code/implementation 
can be easily changed anytime later but not the interface we expose to the 
user. We can have schema revisioning to support more enhancements. But with 
current schema, it will be a lot of behavior change next time if something has 
to be added which will not be good.


- Rohini


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


On Jan. 18, 2016, 1:28 p.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated Jan. 18, 2016, 1:28 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/CallBack.java 
> PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/HTTPCallBack.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSCallBack.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/CallbackActionService.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 0a7e250 
>   
> core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
>  PRE-CREATION 
>   docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
>   docs/src/site/twiki/index.twiki 8591530 
> 
> Diff: https://reviews.apache.org/r/36123/diff/
> 
> 
> Testing
> ---
> 
> Done, 
> Will add more test cases.
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Rohini Palaniswamy]

> On Jan. 18, 2016, 5:31 a.m., Rohini Palaniswamy wrote:
> > client/src/main/resources/callback-action-0.1.xsd, lines 28-31
> > 
> >
> > Would suggest two additions - type and configuration.
> > 
> > url
> > type
> > method
> > configuration
> > arg
> > capture-output
> > 
> > - type can be http or jms. This can be used to abstract the diffent 
> > options supported in different classes and instantiated via factory making 
> > it more clean and extensible. With type, method can then just be GET, PUT 
> > and POST. 
> > - Use configuration instead of arg if it is property with name and 
> > value. 
> > - Can still keep arg to pass arguments like in pig/hive/spark actions 
> > and use Options parser to parse the arguments if needed.
> 
> Jaydeep Vishwakarma wrote:
> In future we can have some other way of sending notification.  GET, PUT 
> and POST cannot be generic,  As these terminology are only meaningful for 
> http calls.
> It does not belongs to configuration category according to it 
> characterstics. 
> I always felt to have more cleaner way for  pig/hive/spark action as 
> well. The current format making it more cleaner. 
> Please let me know if you have diffrent thought.

> In future we can have some other way of sending notification.  GET, PUT and 
> POST cannot be generic,  As these terminology are only meaningful for http 
> calls.
  
  Yes. For that very reason, you definitely need type. When type is defined as 
HTTP, method values will be GET, PUT and POST. That is more clean instead of 
adding multiple different kinds of methods like HTTP_GET, HTTP_PUT, 
QUEUE_PUBLISH and some XXX in future without any relation between them.
  
> I always felt to have more cleaner way for  pig/hive/spark action as well.
  
  Pig/hive/spark/distcp are clean. They have configuration which means actual 
configuration used while running and arg to mean the command line arguments 
they take. It is easy for a commandline user to relate to as there is direct 
correlation. I agree with you configuration does not make sense in this case as 
it is actual data that you are passing there and not configuration. But arg 
does not make sense either as it is not arguments. In traditional sense, 
arguments should be based on commandline options like (-key1 val1 -key2 val2) 
or atleast positional (first argument is is value for key1, second argument is 
value for key2, etc). You can go probably go with params instead.


> On Jan. 18, 2016, 5:31 a.m., Rohini Palaniswamy wrote:
> > docs/src/site/twiki/DG_CallbackActionExtension.twiki, lines 64-66
> > 
> >
> > Currently it only seems to be contents of body of MapMessage. In 
> > future, if support for Message properties has to be added how is that 
> > supposed to be done? Or if different type of Message - says TextMessage has 
> > to be supported.
> > 
> > In case of http get, it seems to be query params and with post it seems 
> > to be form params (application/x-www-form-urlencoded). What if 
> > application/octet-stream needs to be supported in the future? 
> > 
> > I think we need to define and document all this clearly and also make 
> > sure it is not difficult to enhance in future. As I see it, the current 
> > form of definition makes it hard to support additional features of HTTP or 
> > JMS in future without hacking around how arguments are defined as we are 
> > assuming now by default there is only one way of doing it.
> 
> Jaydeep Vishwakarma wrote:
> I agree,  I will detail out all the details in doc. 
> I have created a jira OOZIE-2403 for custom method. I will take care the 
> future enchancement part there.

I am not asking about adding those future enhancements. The current xsd and 
argument/data passing interface is not clean enough to extend in future. That 
needs to be fixed here taking future enhancements into account. Even current 
usage is not even clear from documentation. I had to look into code to 
understand what kind of data and parameters are constructed for all methods and 
what needs to be actually passed in the arg section by the user for them to be 
used.


> On Jan. 18, 2016, 5:31 a.m., Rohini Palaniswamy wrote:
> > core/src/main/resources/oozie-default.xml, line 2577
> > 
> >
> > Can you keep this setting similar to 
> > oozie.jms.producer.connection.properties? It will allow specifying more 
> > properties if needed.
> 
> Jaydeep Vishwakarma wrote:
> oozie.jms.producer.connection.properties have # and ;  to separate 
> multiple properties which is not looking clean from my view. Currently I am 
> not seeing the use of multiple values to provide in jms callback.

Sounds good


- Rohini


-

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

> On Jan. 18, 2016, 5:31 a.m., Rohini Palaniswamy wrote:
> > core/src/main/resources/oozie-default.xml, line 2577
> > 
> >
> > Can you keep this setting similar to 
> > oozie.jms.producer.connection.properties? It will allow specifying more 
> > properties if needed.

oozie.jms.producer.connection.properties have # and ;  to separate multiple 
properties which is not looking clean from my view. Currently I am not seeing 
the use of multiple values to provide in jms callback.


> On Jan. 18, 2016, 5:31 a.m., Rohini Palaniswamy wrote:
> > client/src/main/resources/callback-action-0.1.xsd, lines 28-31
> > 
> >
> > Would suggest two additions - type and configuration.
> > 
> > url
> > type
> > method
> > configuration
> > arg
> > capture-output
> > 
> > - type can be http or jms. This can be used to abstract the diffent 
> > options supported in different classes and instantiated via factory making 
> > it more clean and extensible. With type, method can then just be GET, PUT 
> > and POST. 
> > - Use configuration instead of arg if it is property with name and 
> > value. 
> > - Can still keep arg to pass arguments like in pig/hive/spark actions 
> > and use Options parser to parse the arguments if needed.

In future we can have some other way of sending notification.  GET, PUT and 
POST cannot be generic,  As these terminology are only meaningful for http 
calls.
It does not belongs to configuration category according to it characterstics. 
I always felt to have more cleaner way for  pig/hive/spark action as well. The 
current format making it more cleaner. 
Please let me know if you have diffrent thought.


> On Jan. 18, 2016, 5:31 a.m., Rohini Palaniswamy wrote:
> > docs/src/site/twiki/DG_CallbackActionExtension.twiki, lines 64-66
> > 
> >
> > Currently it only seems to be contents of body of MapMessage. In 
> > future, if support for Message properties has to be added how is that 
> > supposed to be done? Or if different type of Message - says TextMessage has 
> > to be supported.
> > 
> > In case of http get, it seems to be query params and with post it seems 
> > to be form params (application/x-www-form-urlencoded). What if 
> > application/octet-stream needs to be supported in the future? 
> > 
> > I think we need to define and document all this clearly and also make 
> > sure it is not difficult to enhance in future. As I see it, the current 
> > form of definition makes it hard to support additional features of HTTP or 
> > JMS in future without hacking around how arguments are defined as we are 
> > assuming now by default there is only one way of doing it.

I agree,  I will detail out all the details in doc. 
I have created a jira OOZIE-2403 for custom method. I will take care the future 
enchancement part there.


- Jaydeep


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


On Jan. 18, 2016, 1:28 p.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated Jan. 18, 2016, 1:28 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/CallBack.java 
> PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/HTTPCallBack.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSCallBack.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/CallbackActionService.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 0a7e250 
>   
> core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
>  PRE-CREATION 
>   docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
>   docs/src/site/t

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

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

(Updated Jan. 18, 2016, 1:28 p.m.)


Review request for oozie.


Bugs: OOZIE-2259
https://issues.apache.org/jira/browse/OOZIE-2259


Repository: oozie-git


Description
---

Adding callback as an action, It have support for HTTP and JMS server. Not 
covering Excecutor level queue, I will create a separate jira for it.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
  client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/CallBack.java 
PRE-CREATION 
  
core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/HTTPCallBack.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/JMSCallBack.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/service/CallbackActionService.java 
PRE-CREATION 
  core/src/main/resources/oozie-default.xml 0a7e250 
  
core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
 PRE-CREATION 
  docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
  docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
  docs/src/site/twiki/index.twiki 8591530 

Diff: https://reviews.apache.org/r/36123/diff/


Testing
---

Done, 
Will add more test cases.


Thanks,

Jaydeep Vishwakarma

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Rohini Palaniswamy]

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



client/src/main/resources/callback-action-0.1.xsd (lines 28 - 31)


Would suggest two additions - type and configuration.

url
type
method
configuration
arg
capture-output

- type can be http or jms. This can be used to abstract the diffent options 
supported in different classes and instantiated via factory making it more 
clean and extensible. With type, method can then just be GET, PUT and POST. 
- Use configuration instead of arg if it is property with name and value. 
- Can still keep arg to pass arguments like in pig/hive/spark actions and 
use Options parser to parse the arguments if needed.



core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java 
(line 98)


Move all HTTP code to HTTPCallBack and JMS code to JMSCallback classes and 
just call sendNotification on them

interface CallBack

sendNotification(Context, URI, List args, Configuration, boolean 
capture-output)



core/src/main/java/org/apache/oozie/service/CallbackActionService.java (lines 
28 - 30)


Remove invalid comment.



core/src/main/resources/oozie-default.xml (line 2577)


Can you keep this setting similar to 
oozie.jms.producer.connection.properties? It will allow specifying more 
properties if needed.



docs/src/site/twiki/DG_CallbackActionExtension.twiki (lines 64 - 66)


Currently it only seems to be contents of body of MapMessage. In future, if 
support for Message properties has to be added how is that supposed to be done? 
Or if different type of Message - says TextMessage has to be supported.

In case of http get, it seems to be query params and with post it seems to 
be form params (application/x-www-form-urlencoded). What if 
application/octet-stream needs to be supported in the future? 

I think we need to define and document all this clearly and also make sure 
it is not difficult to enhance in future. As I see it, the current form of 
definition makes it hard to support additional features of HTTP or JMS in 
future without hacking around how arguments are defined as we are assuming now 
by default there is only one way of doing it.


- Rohini Palaniswamy


On Nov. 30, 2015, 8:02 p.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated Nov. 30, 2015, 8:02 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/CallbackActionService.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 0a7e250 
>   
> core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
>  PRE-CREATION 
>   docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
>   docs/src/site/twiki/index.twiki 8591530 
> 
> Diff: https://reviews.apache.org/r/36123/diff/
> 
> 
> Testing
> ---
> 
> Done, 
> Will add more test cases.
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

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

(Updated Nov. 30, 2015, 8:02 p.m.)


Review request for oozie.


Changes
---

fixing params for get request


Bugs: OOZIE-2259
https://issues.apache.org/jira/browse/OOZIE-2259


Repository: oozie-git


Description
---

Adding callback as an action, It have support for HTTP and JMS server. Not 
covering Excecutor level queue, I will create a separate jira for it.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
  client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
  
core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/service/CallbackActionService.java 
PRE-CREATION 
  core/src/main/resources/oozie-default.xml 0a7e250 
  
core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
 PRE-CREATION 
  docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
  docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
  docs/src/site/twiki/index.twiki 8591530 

Diff: https://reviews.apache.org/r/36123/diff/


Testing
---

Done, 
Will add more test cases.


Thanks,

Jaydeep Vishwakarma

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

> On Nov. 25, 2015, 9:47 p.m., Robert Kanter wrote:
> > core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java,
> >  line 133
> > 
> >
> > Perhaps we should use ``(freeHTTPPort + 1)`` for this?

I understood your concern. Even it is possible that the freeHTTPPort + 1 has 
been occupied by some server. I will use findFreePort() to ensure the unused 
port.


> On Nov. 25, 2015, 9:47 p.m., Robert Kanter wrote:
> > core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java,
> >  line 172
> > 
> >
> > Perhaps we should use ``(freeJMSPort + 1)`` for this?

Same here , I will use findFreePort() to ensure the unused port.


- Jaydeep


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


On Nov. 26, 2015, 7:43 a.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated Nov. 26, 2015, 7:43 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/CallbackActionService.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 0a7e250 
>   
> core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
>  PRE-CREATION 
>   docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
>   docs/src/site/twiki/index.twiki 8591530 
> 
> Diff: https://reviews.apache.org/r/36123/diff/
> 
> 
> Testing
> ---
> 
> Done, 
> Will add more test cases.
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

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

(Updated Nov. 26, 2015, 7:43 a.m.)


Review request for oozie.


Bugs: OOZIE-2259
https://issues.apache.org/jira/browse/OOZIE-2259


Repository: oozie-git


Description
---

Adding callback as an action, It have support for HTTP and JMS server. Not 
covering Excecutor level queue, I will create a separate jira for it.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
  client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
  
core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/service/CallbackActionService.java 
PRE-CREATION 
  core/src/main/resources/oozie-default.xml 0a7e250 
  
core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
 PRE-CREATION 
  docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
  docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
  docs/src/site/twiki/index.twiki 8591530 

Diff: https://reviews.apache.org/r/36123/diff/


Testing
---

Done, 
Will add more test cases.


Thanks,

Jaydeep Vishwakarma

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Robert Kanter]

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



core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java 
(line 195)


We're in the CallbackActionExecutor, so you can just refer to these 
directly: CALLBACK_ACTION_HTTP_CONNECTION_TIMEOUT_SECS, 
CALLBACK_ACTION_HTTP_SOCKET_TIMEOUT_SECS, and CALLBACK_ACTION_KEEPALIVE_SECS
to improve readability



core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
 (line 133)


Perhaps we should use ``(freeHTTPPort + 1)`` for this?



core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
 (line 172)


Perhaps we should use ``(freeJMSPort + 1)`` for this?



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 12)


Callback Action



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 97)


minOccurs="0".

Please double check that this all matches the XSD file.  It might be 
easiest to just copy-paste it again.


- Robert Kanter


On Nov. 12, 2015, 12:21 p.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated Nov. 12, 2015, 12:21 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/CallbackActionService.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 0a7e250 
>   
> core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
>  PRE-CREATION 
>   docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
>   docs/src/site/twiki/index.twiki 8591530 
> 
> Diff: https://reviews.apache.org/r/36123/diff/
> 
> 
> Testing
> ---
> 
> Done, 
> Will add more test cases.
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

> On Nov. 5, 2015, 11:26 p.m., Robert Kanter wrote:
> > client/src/main/resources/callback-action-0.1.xsd, line 30
> > 
> >
> > Shouldn't we allow minOccurs="0" here?  What if there are no arguments 
> > for the callback?  Or it's somehow encoded in the URL or whatever.

Yes, Puru also mentioned the same. I am taking care of it.


> On Nov. 5, 2015, 11:26 p.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java,
> >  line 64
> > 
> >
> > This is probably too much for this JIRA, so maybe it should go in 
> > another JIRA, but it would be nice if the METHOD could somehow be 
> > pluggable.  For example, say a user has a custom notificaiton system or 
> > uses something other than HTTP or KMS, then they could implement a little 
> > bit of code, add a new METHOD, and use that.  We don't have any existing 
> > action types that are themselves pluggable, and doing that sounds pretty 
> > complicated.  In any case, can you file a JIRA to add this ability?  (You 
> > can leave it unassigned)

Thats nice thought. I will create the jira.


> On Nov. 5, 2015, 11:26 p.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java,
> >  line 265
> > 
> >
> > Perhaps we should implement kill?  Let's say the timeout is set really 
> > long and it's taking a long time to send the callback, the user might want 
> > to kill the action.  I'm not sure if there's a way to "interrupt" an HTTP 
> > call or JMS call.  If not, or if this would be super complicated, then 
> > don't worry about it.

We can abort the http calls. By nature of JMS is asynchronous. Producer 
publishes messages, it doesn't need to wait consumer. I will handle this case 
in https://issues.apache.org/jira/browse/OOZIE-2331.


> On Nov. 5, 2015, 11:26 p.m., Robert Kanter wrote:
> > core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java,
> >  line 60
> > 
> >
> > Will this work with port "0" (i.e. a random open port)?  Otherwise, 
> > this could run into port conflicts.

Agreed with port conflict. Searching for available port and assigning it.


> On Nov. 5, 2015, 11:26 p.m., Robert Kanter wrote:
> > core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java,
> >  line 65
> > 
> >
> > Same.  We should use port "0".

Searching for random port and assigning it .


> On Nov. 5, 2015, 11:26 p.m., Robert Kanter wrote:
> > docs/src/site/twiki/DG_CallbackActionExtension.twiki, line 7
> > 
> >
> > We should be consistent about the capitalization of "Callback".  Is it 
> > "CallBack" or "Callback"?  Personally, I prefer "Callback".  Other parts in 
> > the code and elsewhere had "CallBack".

Yeah "Callback" make more sense.


> On Nov. 5, 2015, 11:26 p.m., Robert Kanter wrote:
> > core/src/main/java/org/apache/oozie/service/CallBackActionService.java, 
> > line 59
> > 
> >
> > Have you tried using a self-signed certificate?  In general, you should 
> > have to pass "-Djavax.net.ssl.trustStore=/path/to/trustore.jks" to the 
> > Oozie Server Tomcat instance.

I have checked it , It is working. I am thinking to keep the trust store 
separate from server, As everytime we add the trust store you need a restart. 
Passing it throght workflow will make more sense, I can take this in another 
jira , what do you say?


- Jaydeep


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


On Nov. 12, 2015, 12:21 p.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated Nov. 12, 2015, 12:21 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   
> core/src/main/java

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

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

(Updated Nov. 12, 2015, 12:21 p.m.)


Review request for oozie.


Bugs: OOZIE-2259
https://issues.apache.org/jira/browse/OOZIE-2259


Repository: oozie-git


Description
---

Adding callback as an action, It have support for HTTP and JMS server. Not 
covering Excecutor level queue, I will create a separate jira for it.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
  client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
  
core/src/main/java/org/apache/oozie/action/callback/CallbackActionExecutor.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/service/CallbackActionService.java 
PRE-CREATION 
  core/src/main/resources/oozie-default.xml 0a7e250 
  
core/src/test/java/org/apache/oozie/action/callback/TestCallbackActionExecutor.java
 PRE-CREATION 
  docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
  docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
  docs/src/site/twiki/index.twiki 8591530 

Diff: https://reviews.apache.org/r/36123/diff/


Testing
---

Done, 
Will add more test cases.


Thanks,

Jaydeep Vishwakarma

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Robert Kanter]

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



client/src/main/resources/callback-action-0.1.xsd (line 30)


Shouldn't we allow minOccurs="0" here?  What if there are no arguments for 
the callback?  Or it's somehow encoded in the URL or whatever.



core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
(line 64)


This is probably too much for this JIRA, so maybe it should go in another 
JIRA, but it would be nice if the METHOD could somehow be pluggable.  For 
example, say a user has a custom notificaiton system or uses something other 
than HTTP or KMS, then they could implement a little bit of code, add a new 
METHOD, and use that.  We don't have any existing action types that are 
themselves pluggable, and doing that sounds pretty complicated.  In any case, 
can you file a JIRA to add this ability?  (You can leave it unassigned)



core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
(line 265)


Perhaps we should implement kill?  Let's say the timeout is set really long 
and it's taking a long time to send the callback, the user might want to kill 
the action.  I'm not sure if there's a way to "interrupt" an HTTP call or JMS 
call.  If not, or if this would be super complicated, then don't worry about it.



core/src/main/java/org/apache/oozie/service/CallBackActionService.java (line 59)


Have you tried using a self-signed certificate?  In general, you should 
have to pass "-Djavax.net.ssl.trustStore=/path/to/trustore.jks" to the Oozie 
Server Tomcat instance.



core/src/main/resources/oozie-default.xml (line 2577)


This is specific for the CallBackAction, right?  Perhaps we should rename 
this to "oozie.action.callback.jms.naming.factory.initial".

And the description should say "CallBack Action", not "jms action".



core/src/main/resources/oozie-default.xml (line 2588)


Better description



core/src/main/resources/oozie-default.xml (line 2596)


Better description



core/src/main/resources/oozie-default.xml (line 2604)


Better description



core/src/main/resources/oozie-default.xml (line 2612)


Better description



core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
 (line 60)


Will this work with port "0" (i.e. a random open port)?  Otherwise, this 
could run into port conflicts.



core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
 (line 65)


Same.  We should use port "0".



core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
 (line 138)


There should be a call to ``fail("reason")`` in the try block so that if 
there's no Exception, the test will fail.



core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
 (line 175)


There should be a call to ``fail("reason")`` in the try block so that if 
there's no Exception, the test will fail.



core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
 (line 194)


There should be a call to ``fail("reason")`` in the try block so that if 
there's no Exception, the test will fail.



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 7)


We should be consistent about the capitalization of "Callback".  Is it 
"CallBack" or "Callback"?  Personally, I prefer "Callback".  Other parts in the 
code and elsewhere had "CallBack".



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 12)


3.2.4?



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 15)


=http(s)=



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 23)


Let's use a newer version of the workflow schema, (e.g. 0.4 or 0.5)



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 26)


The xmlns here is wrong



docs/src/sit

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Srikanth Sundarrajan]

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

Ship it!


+1. Am assuming as discussed on the jira thread, a separate issue would be 
filed to move the local action execution into an independent thread pool.

- Srikanth Sundarrajan


On Aug. 12, 2015, 11:36 a.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated Aug. 12, 2015, 11:36 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/CallBackActionService.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 0a7e250 
>   
> core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
>  PRE-CREATION 
>   docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
>   docs/src/site/twiki/index.twiki 8591530 
> 
> Diff: https://reviews.apache.org/r/36123/diff/
> 
> 
> Testing
> ---
> 
> Done, 
> Will add more test cases.
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

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

(Updated Aug. 12, 2015, 11:36 a.m.)


Review request for oozie.


Changes
---

Rebased the patch, fixing the property names


Bugs: OOZIE-2259
https://issues.apache.org/jira/browse/OOZIE-2259


Repository: oozie-git


Description
---

Adding callback as an action, It have support for HTTP and JMS server. Not 
covering Excecutor level queue, I will create a separate jira for it.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/OozieCLI.java 48bac7d 
  client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
  
core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/service/CallBackActionService.java 
PRE-CREATION 
  core/src/main/resources/oozie-default.xml 0a7e250 
  
core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
 PRE-CREATION 
  docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
  docs/src/site/twiki/DG_CommandLineTool.twiki 1823247 
  docs/src/site/twiki/index.twiki 8591530 

Diff: https://reviews.apache.org/r/36123/diff/


Testing
---

Done, 
Will add more test cases.


Thanks,

Jaydeep Vishwakarma

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

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

(Updated Aug. 11, 2015, 12:53 p.m.)


Review request for oozie.


Bugs: OOZIE-2259
https://issues.apache.org/jira/browse/OOZIE-2259


Repository: oozie-git


Description
---

Adding callback as an action, It have support for HTTP and JMS server. Not 
covering Excecutor level queue, I will create a separate jira for it.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/OozieCLI.java c381432 
  client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
  
core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/service/CallBackActionService.java 
PRE-CREATION 
  core/src/main/resources/oozie-default.xml 4dc127e 
  
core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
 PRE-CREATION 
  docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
  docs/src/site/twiki/DG_CommandLineTool.twiki 9494b22 
  docs/src/site/twiki/index.twiki 8591530 

Diff: https://reviews.apache.org/r/36123/diff/


Testing
---

Done, 
Will add more test cases.


Thanks,

Jaydeep Vishwakarma

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Srikanth Sundarrajan]

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



core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
(line 137)


should be >= 400



core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
(line 53)


sends notification along with relevant arguments to applicable end point.



core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
(line 61)


Would like constant and the config name to include the units (for ex. if 
the units are in milliseconds), would prefer the constant to be named 
CALLBACK_ACTION_HTTP_SOCKET_TIMEOUT_MS and the name to be 
oozie.action.callback.http.socket.timeout.ms



core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
 (line 22)


You can't have dependency on com.sun.* they are typically private and 
shouldn't be used.



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 14)


It has support for



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 15)


Replace "An callback action" with "Callback action"



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 56)


Default values for callback action properties are configured in 
oozie-default.xml and can be overriden through oozie-site.xml


- Srikanth Sundarrajan


On Aug. 6, 2015, 9:13 a.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated Aug. 6, 2015, 9:13 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java c381432 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/CallBackActionService.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 4dc127e 
>   
> core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
>  PRE-CREATION 
>   docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 9494b22 
>   docs/src/site/twiki/index.twiki 8591530 
> 
> Diff: https://reviews.apache.org/r/36123/diff/
> 
> 
> Testing
> ---
> 
> Done, 
> Will add more test cases.
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

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

(Updated Aug. 6, 2015, 9:13 a.m.)


Review request for oozie.


Changes
---

http connections were not closing properly. Resolved it now. added socket and 
connection time out as well for http methods.


Bugs: OOZIE-2259
https://issues.apache.org/jira/browse/OOZIE-2259


Repository: oozie-git


Description
---

Adding callback as an action, It have support for HTTP and JMS server. Not 
covering Excecutor level queue, I will create a separate jira for it.


Diffs (updated)
-

  client/src/main/java/org/apache/oozie/cli/OozieCLI.java c381432 
  client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
  
core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/service/CallBackActionService.java 
PRE-CREATION 
  core/src/main/resources/oozie-default.xml 4dc127e 
  
core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
 PRE-CREATION 
  docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
  docs/src/site/twiki/DG_CommandLineTool.twiki 9494b22 
  docs/src/site/twiki/index.twiki 8591530 

Diff: https://reviews.apache.org/r/36123/diff/


Testing
---

Done, 
Will add more test cases.


Thanks,

Jaydeep Vishwakarma

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

> On July 14, 2015, 8:26 a.m., Srikanth Sundarrajan wrote:
> > core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java,
> >  line 200
> > 
> >
> > There can be other params present here. Check for 2 elements may not be 
> > appropriate

I am spliting it with "?" and then checking the length, In what condition url 
can have more than one "?". I am also assuming this will have the queue/topic 
along with url only.


> On July 14, 2015, 8:26 a.m., Srikanth Sundarrajan wrote:
> > core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java,
> >  line 62
> > 
> >
> > rename to CALLBACK_ACTION_TIMEOUT_MS and also rename the conf value to 
> > include ms

As it handle Seconds , I am changing it to CALLBACK_ACTION_TIMEOUT_SECONDS


- Jaydeep


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


On July 9, 2015, 11:51 a.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated July 9, 2015, 11:51 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java c381432 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/CallBackActionService.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 4dc127e 
>   
> core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
>  PRE-CREATION 
>   docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 9494b22 
>   docs/src/site/twiki/index.twiki 8591530 
> 
> Diff: https://reviews.apache.org/r/36123/diff/
> 
> 
> Testing
> ---
> 
> Done, 
> Will add more test cases.
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Srikanth Sundarrajan]

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



core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
(line 57)


rename to CALLBACK_ACTION_TIMEOUT_MS and also rename the conf value to 
include ms



core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
(line 121)


== can be used for enum comparisons



core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
(line 193)


There can be other params present here. Check for 2 elements may not be 
appropriate



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 56)


reword



docs/src/site/twiki/DG_CallbackActionExtension.twiki (line 84)


overwritten


- Srikanth Sundarrajan


On July 9, 2015, 11:51 a.m., Jaydeep Vishwakarma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36123/
> ---
> 
> (Updated July 9, 2015, 11:51 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2259
> https://issues.apache.org/jira/browse/OOZIE-2259
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> Adding callback as an action, It have support for HTTP and JMS server. Not 
> covering Excecutor level queue, I will create a separate jira for it.
> 
> 
> Diffs
> -
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java c381432 
>   client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
>   
> core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
> PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/CallBackActionService.java 
> PRE-CREATION 
>   core/src/main/resources/oozie-default.xml 4dc127e 
>   
> core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
>  PRE-CREATION 
>   docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 9494b22 
>   docs/src/site/twiki/index.twiki 8591530 
> 
> Diff: https://reviews.apache.org/r/36123/diff/
> 
> 
> Testing
> ---
> 
> Done, 
> Will add more test cases.
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>

Re: Review Request 36123: OOZIE-2259 : Callback Action Executor

[Jaydeep Vishwakarma]

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

(Updated July 9, 2015, 11:51 a.m.)


Review request for oozie.


Bugs: OOZIE-2259
https://issues.apache.org/jira/browse/OOZIE-2259


Repository: oozie-git


Description
---

Adding callback as an action, It have support for HTTP and JMS server. Not 
covering Excecutor level queue, I will create a separate jira for it.


Diffs
-

  client/src/main/java/org/apache/oozie/cli/OozieCLI.java c381432 
  client/src/main/resources/callback-action-0.1.xsd PRE-CREATION 
  
core/src/main/java/org/apache/oozie/action/callback/CallBackActionExecutor.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/action/callback/JMSNotification.java 
PRE-CREATION 
  core/src/main/java/org/apache/oozie/service/CallBackActionService.java 
PRE-CREATION 
  core/src/main/resources/oozie-default.xml 4dc127e 
  
core/src/test/java/org/apache/oozie/action/callback/TestCallBackActionExecutor.java
 PRE-CREATION 
  docs/src/site/twiki/DG_CallbackActionExtension.twiki PRE-CREATION 
  docs/src/site/twiki/DG_CommandLineTool.twiki 9494b22 
  docs/src/site/twiki/index.twiki 8591530 

Diff: https://reviews.apache.org/r/36123/diff/


Testing
---

Done, 
Will add more test cases.


Thanks,

Jaydeep Vishwakarma