> 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. > > 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 <job-tracker> 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 test cases. > > > Thanks, > > Jaydeep Vishwakarma > >
