> On Jan. 18, 2016, 5:31 a.m., Rohini Palaniswamy wrote:
> > core/src/main/resources/oozie-default.xml, line 2577
> > <https://reviews.apache.org/r/36123/diff/8/?file=1148600#file1148600line2577>
> >
> >     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
> > <https://reviews.apache.org/r/36123/diff/8/?file=1148596#file1148596line28>
> >
> >     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
> > <https://reviews.apache.org/r/36123/diff/8/?file=1148602#file1148602line64>
> >
> >     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/twiki/index.twiki 8591530 
> 
> Diff: https://reviews.apache.org/r/36123/diff/
> 
> 
> Testing
> -------
> 
> Done, 
> Will add more test cases.
> 
> 
> Thanks,
> 
> Jaydeep Vishwakarma
> 
>

Reply via email to