Automatically inject Azkaban properties prefixed with hadoop-inject. into ...
#164
Conversation
…to the Hadoop configuration for HadoopJava, Pig, Hive and Java job types
| @@ -109,6 +157,13 @@ public static String getDirName(Props jobProps) { | |||
| return "_link_" + dirSuffix; | |||
There was a problem hiding this comment.
Perhaps we should name this directory _resources_...?
There was a problem hiding this comment.
I agree that resources would be better, but let's just leave it, so that we have a 0% chance of breaking anything that is already working.
There was a problem hiding this comment.
These are written out per execution, so unless people are running with crazy mixed classpaths, this should be fine to change.
There was a problem hiding this comment.
Fixed - changed to "resources" as suggested.
|
+1 This is great! I think it will make users' lives much easier! |
|
@hluu, @wagnermarkd, @logiclord, any concerns before merging? See internal JIRA HADOOP-7962 for details. |
|
Let me take a look. |
| public static void prepareConf(Props props, String workingDir) { | ||
| try { | ||
| Configuration conf = new Configuration(false); | ||
| String confPrefix = "hadoop-conf."; |
There was a problem hiding this comment.
Can we name it something other than hadoop-conf? Something that indicates that these are generated configurations coming from azkaban would be good.
…nto the Hadoop conf automatically
| public static void injectResources() { | ||
| Configuration.addDefaultResource(azkabanInjectFile); | ||
| Configuration.addDefaultResource(azkabanLinksFile); | ||
| } |
There was a problem hiding this comment.
Seems like there's an extra space in front of the }.
|
LGTM. @hluu, @wagnermarkd -- any final comments before we merge? |
|
The code looks good. |
|
Now that we can a generic way of injecting Azkaban properties with "azkaban-inject" prefix into Hadoop configurations, it will be more clean to combine "azkaban-inject.xml" and "azkaban-links.xml" into a single file. Method prepareLinks() can just populate those properties with "azkaban-inject" prefix and let method prepareConf() write them out. |
…ies into one single code path for the HadoopConfigurationInjector, based on Hien's pull request feedback
|
@hluu I have tested locally to make sure the resource file is being written correctly and that all the properties are being injected into the Hadoop configuration. Since I don't have a local cluster up, I can't check that the Hadoop conf that's actually being sent to all the tasks actually looks right, but it should be good. For your comments: |
| * Writes out the XML configuration file that will be injected by the client | ||
| * as a configuration resource. | ||
| * <p> | ||
| * This will file include a series of links injected by Azkaban as well as |
There was a problem hiding this comment.
This will file --> This file will
|
LGTM. @hluu, if you're okay with this latest version, please go ahead and merge this pull request. |
|
LGTM |
Automatically inject Azkaban properties beginning with hadoop-conf into ...
erwa
changed the title
hadoop-inject. into ...
...the Hadoop configuration for HadoopJava, Pig, Hive and Java job types