Hadoop QA
added a comment - 18/Apr/12 15:35 +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12523188/MAPREDUCE-4161-1.patch
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 2 new or modified test files.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in .
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2247//testReport/
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2247//console
This message is automatically generated.

Not done with the patch yet. It looks good, but I have one minor comment about some of the changes. In a few places you replaced the creation of a YarnConfiguration object with just a configuration object. Yes most of the time this should really be a noop, but YarnConfiguration does some static initialization, that I have seen issues in the past where if that was not done in the correct order bad things happened. I am just being paranoid here.

Robert Joseph Evans
added a comment - 18/Apr/12 15:56 Not done with the patch yet. It looks good, but I have one minor comment about some of the changes. In a few places you replaced the creation of a YarnConfiguration object with just a configuration object. Yes most of the time this should really be a noop, but YarnConfiguration does some static initialization, that I have seen issues in the past where if that was not done in the correct order bad things happened. I am just being paranoid here.