steveloughran commented on code in PR #8331:
URL: https://github.com/apache/hadoop/pull/8331#discussion_r3336342503
##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/main/java/org/apache/hadoop/mapred/YARNRunner.java:
##########
@@ -503,7 +503,9 @@ private List<String> setupAMCommand(Configuration jobConf) {
}
}
- vargs.add(MRJobConfig.APPLICATION_MASTER_CLASS);
+ String amClass = jobConf.get("yarn.app.mapreduce.am",
+ MRJobConfig.APPLICATION_MASTER_CLASS);
+ vargs.add(amClass);
Review Comment:
Would you want to do any preflight checking here?
we can't look for the class on the CP as it may arrive in the referenced
jar, but should fail fast if the classname is null/""
##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/main/java/org/apache/hadoop/mapred/YARNRunner.java:
##########
@@ -503,7 +503,9 @@ private List<String> setupAMCommand(Configuration jobConf) {
}
}
- vargs.add(MRJobConfig.APPLICATION_MASTER_CLASS);
+ String amClass = jobConf.get("yarn.app.mapreduce.am",
Review Comment:
use getTrimmed() to trim whitespace
##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/main/java/org/apache/hadoop/mapred/YARNRunner.java:
##########
@@ -503,7 +503,9 @@ private List<String> setupAMCommand(Configuration jobConf) {
}
}
- vargs.add(MRJobConfig.APPLICATION_MASTER_CLASS);
+ String amClass = jobConf.get("yarn.app.mapreduce.am",
Review Comment:
I concur with everything @K0K0V0K says.
- public string constant
- updated default file
- doc change.
Test wise,
1. Create a subclass of the normal MR AM which does something observable in
startup (touch a file on a configured path?). Launch a minimal MR job and
assert the file exists.
2. Launch an MR job with a configuration that refers to a class which isn't
present. Expect failure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
