Addressing code review comments by rkanter Change-Id: I76aa2549923b69959e32db1630914bc3f4450c27
Project: http://git-wip-us.apache.org/repos/asf/oozie/repo Commit: http://git-wip-us.apache.org/repos/asf/oozie/commit/77cd3f4e Tree: http://git-wip-us.apache.org/repos/asf/oozie/tree/77cd3f4e Diff: http://git-wip-us.apache.org/repos/asf/oozie/diff/77cd3f4e Branch: refs/heads/oya Commit: 77cd3f4eba931a7f282e2bd4cb277e8d965a64fc Parents: 8c4a856 Author: Peter Bacsko <pbac...@cloudera.com> Authored: Fri Jan 20 13:06:42 2017 +0100 Committer: Peter Bacsko <pbac...@cloudera.com> Committed: Fri Jan 20 13:06:42 2017 +0100 ---------------------------------------------------------------------- .../oozie/action/hadoop/JavaActionExecutor.java | 2 +- .../action/hadoop/MapReduceActionExecutor.java | 22 +------------------- .../action/hadoop/SqoopActionExecutor.java | 2 +- .../hadoop/TestMapReduceActionExecutor.java | 20 ++++-------------- 4 files changed, 7 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/oozie/blob/77cd3f4e/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java b/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java index 9114da8..4b9b16d 100644 --- a/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java +++ b/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java @@ -1306,7 +1306,7 @@ public class JavaActionExecutor extends ActionExecutor { * @return JobClient * @throws HadoopAccessorException */ - protected JobClient createJobClient(Context context, JobConf jobConf) throws HadoopAccessorException { + protected JobClient createJobClient(Context context, Configuration jobConf) throws HadoopAccessorException { String user = context.getWorkflow().getUser(); return Services.get().get(HadoopAccessorService.class).createJobClient(user, jobConf); } http://git-wip-us.apache.org/repos/asf/oozie/blob/77cd3f4e/core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java b/core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java index 918a269..72695f3 100644 --- a/core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java +++ b/core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java @@ -210,7 +210,7 @@ public class MapReduceActionExecutor extends JavaActionExecutor { if (action.getStatus() == WorkflowAction.Status.OK) { Element actionXml = XmlUtils.parseXml(action.getConf()); Configuration jobConf = createBaseHadoopConf(context, actionXml); - jobClient = createJobClient(context, new JobConf(jobConf)); + jobClient = createJobClient(context, jobConf); RunningJob runningJob = jobClient.getJob(JobID.forName(action.getExternalChildIDs())); if (runningJob == null) { throw new ActionExecutorException(ActionExecutorException.ErrorType.FAILED, "MR002", @@ -296,26 +296,6 @@ public class MapReduceActionExecutor extends JavaActionExecutor { return (actionXml.getChild("streaming", ns) != null) ? "mapreduce-streaming" : null; } - @Override - Configuration createLauncherConf(FileSystem actionFs, Context context, WorkflowAction action, Element actionXml, - Configuration actionConf) throws ActionExecutorException { - // If the user is using a regular MapReduce job and specified an uber jar, we need to also set it for the launcher; - // so we override createLauncherConf to call super and then to set the uber jar if specified. At this point, checking that - // uber jars are enabled and resolving the uber jar path is already done by setupActionConf() when it parsed the actionConf - // argument and we can just look up the uber jar in the actionConf argument. - Configuration launcherJobConf = super.createLauncherConf(actionFs, context, action, actionXml, actionConf); - Namespace ns = actionXml.getNamespace(); - if (actionXml.getChild("streaming", ns) == null && actionXml.getChild("pipes", ns) == null) { - // Set for uber jar - String uberJar = actionConf.get(MapReduceMain.OOZIE_MAPREDUCE_UBER_JAR); - if (uberJar != null && uberJar.trim().length() > 0) { - // TODO - // launcherJobConf.setJar(uberJar); - } - } - return launcherJobConf; - } - public static void setStreaming(Configuration conf, String mapper, String reducer, String recordReader, String[] recordReaderMapping, String[] env) { if (mapper != null) { http://git-wip-us.apache.org/repos/asf/oozie/blob/77cd3f4e/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java b/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java index 7e2561d..1541a42 100644 --- a/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java +++ b/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java @@ -131,7 +131,7 @@ public class SqoopActionExecutor extends JavaActionExecutor { if (action.getStatus() == WorkflowAction.Status.OK) { Element actionXml = XmlUtils.parseXml(action.getConf()); Configuration jobConf = createBaseHadoopConf(context, actionXml); - jobClient = createJobClient(context, new JobConf(jobConf)); + jobClient = createJobClient(context, jobConf); // Cumulative counters for all Sqoop mapreduce jobs Counters counters = null; http://git-wip-us.apache.org/repos/asf/oozie/blob/77cd3f4e/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java ---------------------------------------------------------------------- diff --git a/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java b/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java index 1643bef..bb0a366 100644 --- a/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java +++ b/sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java @@ -250,45 +250,33 @@ public class TestMapReduceActionExecutor extends ActionExecutorTestCase { actionXml = createUberJarActionXML(getNameNodeUri() + "/app/job.jar", ""); conf = ae.createBaseHadoopConf(context, actionXml); ae.setupActionConf(conf, context, actionXml, getFsTestCaseDir()); - assertEquals(getNameNodeUri() + "/app/job.jar", conf.get("oozie.mapreduce.uber.jar")); // absolute path with namenode - launcherJobConf = ae.createLauncherConf(getFileSystem(), context, action, actionXml, conf); - // assertEquals(getNameNodeUri() + "/app/job.jar", launcherJobConf.getJar()); // same for launcher conf + assertEquals(getNameNodeUri() + "/app/job.jar", conf.get(MapReduceMain.OOZIE_MAPREDUCE_UBER_JAR)); // absolute path with namenode actionXml = createUberJarActionXML("/app/job.jar", ""); conf = ae.createBaseHadoopConf(context, actionXml); ae.setupActionConf(conf, context, actionXml, getFsTestCaseDir()); - assertEquals(getNameNodeUri() + "/app/job.jar", conf.get("oozie.mapreduce.uber.jar")); // absolute path without namenode - launcherJobConf = ae.createLauncherConf(getFileSystem(), context, action, actionXml, conf); - // assertEquals(getNameNodeUri() + "/app/job.jar", launcherJobConf.getJar()); // same for launcher conf + assertEquals(getNameNodeUri() + "/app/job.jar", conf.get(MapReduceMain.OOZIE_MAPREDUCE_UBER_JAR)); // absolute path without namenode actionXml = createUberJarActionXML("job.jar", ""); conf = ae.createBaseHadoopConf(context, actionXml); ae.setupActionConf(conf, context, actionXml, getFsTestCaseDir()); - assertEquals(getFsTestCaseDir() + "/job.jar", conf.get("oozie.mapreduce.uber.jar")); // relative path - launcherJobConf = ae.createLauncherConf(getFileSystem(), context, action, actionXml, conf); - // assertEquals(getFsTestCaseDir() + "/job.jar", launcherJobConf.getJar()); // same for launcher + assertEquals(getFsTestCaseDir() + "/job.jar", conf.get(MapReduceMain.OOZIE_MAPREDUCE_UBER_JAR)); // relative path actionXml = createUberJarActionXML("job.jar", "<streaming></streaming>"); conf = ae.createBaseHadoopConf(context, actionXml); ae.setupActionConf(conf, context, actionXml, getFsTestCaseDir()); - assertEquals("", conf.get("oozie.mapreduce.uber.jar")); // ignored for streaming - launcherJobConf = ae.createLauncherConf(getFileSystem(), context, action, actionXml, conf); - // assertNull(launcherJobConf.getJar()); // same for launcher conf (not set) + assertEquals("", conf.get(MapReduceMain.OOZIE_MAPREDUCE_UBER_JAR)); // ignored for streaming actionXml = createUberJarActionXML("job.jar", "<pipes></pipes>"); conf = ae.createBaseHadoopConf(context, actionXml); ae.setupActionConf(conf, context, actionXml, getFsTestCaseDir()); assertEquals("", conf.get("oozie.mapreduce.uber.jar")); // ignored for pipes - launcherJobConf = ae.createLauncherConf(getFileSystem(), context, action, actionXml, conf); - // assertNull(launcherJobConf.getJar()); // same for launcher conf (not set) actionXml = XmlUtils.parseXml("<map-reduce>" + "<job-tracker>" + getJobTrackerUri() + "</job-tracker>" + "<name-node>" + getNameNodeUri() + "</name-node>" + "</map-reduce>"); conf = ae.createBaseHadoopConf(context, actionXml); ae.setupActionConf(conf, context, actionXml, getFsTestCaseDir()); assertNull(conf.get("oozie.mapreduce.uber.jar")); // doesn't resolve if not set - launcherJobConf = ae.createLauncherConf(getFileSystem(), context, action, actionXml, conf); - // assertNull(launcherJobConf.getJar()); // same for launcher conf // Disable uber jars to test that MapReduceActionExecutor won't allow the oozie.mapreduce.uber.jar property serv.getConf().setBoolean("oozie.action.mapreduce.uber.jar.enable", false);
