Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3.0
    • Fix Version/s: 4.0.0
    • Component/s: client, workflow
    • Labels:
      None

      Description

      add support for 'oozie hive' in the CLI

      1. oozie-894.patch
        43 kB
        Bowen Zhang
      2. oozie-894.patch
        43 kB
        Bowen Zhang
      3. oozie-894.patch
        46 kB
        Bowen Zhang

        Issue Links

          Activity

          Hide
          jun aoki added a comment -

          Hi Bowen, are you actively looking into this issue?
          I'm kinda interested but would only watch it if you are. Let me know!

          Show
          jun aoki added a comment - Hi Bowen, are you actively looking into this issue? I'm kinda interested but would only watch it if you are. Let me know!
          Hide
          Bowen Zhang added a comment -

          Hi, I am working on it right now. Thanks.

          Show
          Bowen Zhang added a comment - Hi, I am working on it right now. Thanks.
          Hide
          jun aoki added a comment -

          Bowen, thank you for letting me know. I'm looking forward to seeing your patch!

          Show
          jun aoki added a comment - Bowen, thank you for letting me know. I'm looking forward to seeing your patch!
          Hide
          Bowen Zhang added a comment -

          I think we need have a discussion of whether we want a "-X param_file params" options for Hive CLI. For pig, we have this option described here:http://oozie.apache.org/docs/3.2.0-incubating/DG_CommandLineTool.html#Pig_Operations. However, I think the wiki documentation is confusing as of how to use the "-X" argument and what is the param_file and what should be an example format for params file. Anyone has an input?

          Show
          Bowen Zhang added a comment - I think we need have a discussion of whether we want a "-X param_file params" options for Hive CLI. For pig, we have this option described here: http://oozie.apache.org/docs/3.2.0-incubating/DG_CommandLineTool.html#Pig_Operations . However, I think the wiki documentation is confusing as of how to use the "-X" argument and what is the param_file and what should be an example format for params file. Anyone has an input?
          Hide
          Bowen Zhang added a comment -

          I uploaded a patch for this ticket.

          Show
          Bowen Zhang added a comment - I uploaded a patch for this ticket.
          Hide
          jun aoki added a comment -

          Hi Bowen,

          A few things
          1. ScriptLanguageCommand "method" in OozieCLI should start with "s" instead of "S"
          2. In DagEngine it has "*" in an import statement. Is it a good coding convention?
          3. It tends to combine hive with pig as "ScriptLanguage". I am not too sure we should combine them together. One good place I found it is not good is in TestWorkflowXClient.java

          wc.submitPig(conf, pigScriptFile, null)); // CURRENT WAY
          wc.submitScriptLanguage(conf, pigScriptFile, null, "pig")); // IN THE PATCH
          

          submitPig is a typed method whereas submitScriptLaunguage can take a string in the 4th param, which is error prone.

          Also my 2 cents, it is a big patch so maybe good idea to use reviewboard.

          Show
          jun aoki added a comment - Hi Bowen, A few things 1. ScriptLanguageCommand "method" in OozieCLI should start with "s" instead of "S" 2. In DagEngine it has "*" in an import statement. Is it a good coding convention? 3. It tends to combine hive with pig as "ScriptLanguage". I am not too sure we should combine them together. One good place I found it is not good is in TestWorkflowXClient.java wc.submitPig(conf, pigScriptFile, null )); // CURRENT WAY wc.submitScriptLanguage(conf, pigScriptFile, null , "pig" )); // IN THE PATCH submitPig is a typed method whereas submitScriptLaunguage can take a string in the 4th param, which is error prone. Also my 2 cents, it is a big patch so maybe good idea to use reviewboard.
          Hide
          Bowen Zhang added a comment -

          Looks like review board is down. So, let's just review it here.

          Show
          Bowen Zhang added a comment - Looks like review board is down. So, let's just review it here.
          Hide
          Bowen Zhang added a comment -

          The review board for oozie keeps throwing out 500 server error when creating review request.

          Show
          Bowen Zhang added a comment - The review board for oozie keeps throwing out 500 server error when creating review request.
          Hide
          Alejandro Abdelnur added a comment -

          Thanks Bowen, overall seems in good shape. Following my feedback.

          • patch needs to be rebased, it does not apply cleanly
          • OozieCLI.java, createScriptLanguageOptions()
            • local var 'File' should be 'file'
            • the logic to set pig or hive is a bit odd, it seems it would more natural to pass the description as parameter, then no need to do an IF
          • PigActionExecutor, why the scriptName static var? we can use directly the PIG_SCRIPT constant.
          • HiveActionExecutor, why the scriptName static var? we can use directly the HIVE_SCRIPT constant.
          • ScriptLanguageActionExecutor
            • class should be abstract with getScriptName() method abstract
            • comments/messages should state filesystem instead hdfs
            • exception message should print the path that failed
          • TestWorkflowXCLient, it should have a testSubmitHive() test
          • V1JobsServlet
            • we should use constants instead literals for pig/mapreduce/hive
            • we should put pig/mapreduce/hive in a Set<String> and do contains() instead the current IF statement
          • XOozieClient, submitScripLanguage(), same as in OozieCLI, the logic is a bit odd (setting PIG and then checking if Hive and reseting)

          Finally, it seems it is not possible to do a submit patch for this JIRA (in order to get test-patch to run), if you cannot do that, then open a new JIRA and close this one as DUP.

          Show
          Alejandro Abdelnur added a comment - Thanks Bowen, overall seems in good shape. Following my feedback. patch needs to be rebased, it does not apply cleanly OozieCLI.java, createScriptLanguageOptions() local var 'File' should be 'file' the logic to set pig or hive is a bit odd, it seems it would more natural to pass the description as parameter, then no need to do an IF PigActionExecutor, why the scriptName static var? we can use directly the PIG_SCRIPT constant. HiveActionExecutor, why the scriptName static var? we can use directly the HIVE_SCRIPT constant. ScriptLanguageActionExecutor class should be abstract with getScriptName() method abstract comments/messages should state filesystem instead hdfs exception message should print the path that failed TestWorkflowXCLient, it should have a testSubmitHive() test V1JobsServlet we should use constants instead literals for pig/mapreduce/hive we should put pig/mapreduce/hive in a Set<String> and do contains() instead the current IF statement XOozieClient, submitScripLanguage(), same as in OozieCLI, the logic is a bit odd (setting PIG and then checking if Hive and reseting) Finally, it seems it is not possible to do a submit patch for this JIRA (in order to get test-patch to run), if you cannot do that, then open a new JIRA and close this one as DUP.
          Hide
          Alejandro Abdelnur added a comment -

          Patch looks good, still we don't have a test-patch run. Bowen, please open a new JIRA and attach the latest patch there just for the purposes of test-patch.

          +1 pending jenkins.

          Show
          Alejandro Abdelnur added a comment - Patch looks good, still we don't have a test-patch run. Bowen, please open a new JIRA and attach the latest patch there just for the purposes of test-patch. +1 pending jenkins.
          Hide
          Bowen Zhang added a comment -
          Show
          Bowen Zhang added a comment - patch uploaded at https://issues.apache.org/jira/browse/OOZIE-1258
          Hide
          Alejandro Abdelnur added a comment -

          jenkins test patch is OK. +1. Committing this momentarily.

          Bowen, one thing we've missed is updating the docs for this, would you follow up with a JIRA updating the docs adding something similar to what we have for pig submission? ( http://oozie.apache.org/docs/3.3.1/DG_CommandLineTool.html#Pig_Operations )

          Show
          Alejandro Abdelnur added a comment - jenkins test patch is OK. +1. Committing this momentarily. Bowen, one thing we've missed is updating the docs for this, would you follow up with a JIRA updating the docs adding something similar to what we have for pig submission? ( http://oozie.apache.org/docs/3.3.1/DG_CommandLineTool.html#Pig_Operations )
          Hide
          Alejandro Abdelnur added a comment -

          Thanks Bowen. Committed to trunk.

          Show
          Alejandro Abdelnur added a comment - Thanks Bowen. Committed to trunk.
          Hide
          Virag Kothari added a comment -

          This breaks backward compatibility for clients using submitPig()
          We should deprecate this method before removing it.

          Show
          Virag Kothari added a comment - This breaks backward compatibility for clients using submitPig() We should deprecate this method before removing it.
          Hide
          Robert Kanter added a comment -

          I'll add back the submitPig method and mark it as deprecated in the patch for OOZIE-1353

          Show
          Robert Kanter added a comment - I'll add back the submitPig method and mark it as deprecated in the patch for OOZIE-1353
          Hide
          Virag Kothari added a comment -

          Thanks for doing it

          Show
          Virag Kothari added a comment - Thanks for doing it

            People

            • Assignee:
              Bowen Zhang
              Reporter:
              Alejandro Abdelnur
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development