----------------------------------------------------------- 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) <https://reviews.apache.org/r/36123/#comment175769> 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) <https://reviews.apache.org/r/36123/#comment175772> Move all HTTP code to HTTPCallBack and JMS code to JMSCallback classes and just call sendNotification on them interface CallBack sendNotification(Context, URI, List<String> args, Configuration, boolean capture-output) core/src/main/java/org/apache/oozie/service/CallbackActionService.java (lines 28 - 30) <https://reviews.apache.org/r/36123/#comment175776> Remove invalid comment. core/src/main/resources/oozie-default.xml (line 2577) <https://reviews.apache.org/r/36123/#comment175778> 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) <https://reviews.apache.org/r/36123/#comment175786> 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 > >
