[
https://issues.apache.org/jira/browse/YARN-2369?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14680872#comment-14680872
]
Jason Lowe commented on YARN-2369:
----------------------------------
Sorry for the long delay, and thanks for your patience. Comments on the latest
patch:
The newline after the Configuration.getTrimmedStringCollection was accidentally
removed
There's a lot of unnecessary import changes in MRApps including conversions to
using wildcard imports which are unrelated to this JIRA.
An entry for mapreduce.application.appendable.env.variables should be in
mapred-default.xml with the appropriate default value and description for
documentation purposes
"ClASSPATH" s/b "CLASSPATH" in the unit test comment
The unit test is making an implicit assumption that MRApps.setClasspath is
setting the environment variable iteratively, which is not a requirement for
how it performs semantically. That could change in the future and break this
test. A better unit test would be one that explicitly verifies
MRapps.addtoEnvironment behavior directly by checking for path append behavior
on variables in the configured append list and overwrite behavior for variables
not in that list.
I think the "public static final" should go back on the MRJobConfig additions
to make it consistent with the rest of the file. Checkstyle will complain, but
for this instance I think consistency is better here given the benign nature of
what it is complaining about.
Arrays.asList seems like simpler way to statically initialize the default value
in MRJobConfig as there's no need for the anonymous class, serial version
boilerplate, or invocation of add methods in a static block.
The "/**" line at the end of the MRJobConfig patch hunk had its indentation
accidentally removed.
I'm wondering if we should move this JIRA to the MAPREDUCE project since that's
where most of the changes are and where most of the user-visible functionality
appears. The extreme case is to file three separate for the three affected
projects, but that seems extreme for this case. Any opinions on that?
> Environment variable handling assumes values should be appended
> ---------------------------------------------------------------
>
> Key: YARN-2369
> URL: https://issues.apache.org/jira/browse/YARN-2369
> Project: Hadoop YARN
> Issue Type: Bug
> Affects Versions: 2.2.0
> Reporter: Jason Lowe
> Assignee: Dustin Cote
> Attachments: YARN-2369-1.patch, YARN-2369-2.patch, YARN-2369-3.patch,
> YARN-2369-4.patch, YARN-2369-5.patch, YARN-2369-6.patch
>
>
> When processing environment variables for a container context the code
> assumes that the value should be appended to any pre-existing value in the
> environment. This may be desired behavior for handling path-like environment
> variables such as PATH, LD_LIBRARY_PATH, CLASSPATH, etc. but it is a
> non-intuitive and harmful way to handle any variable that does not have
> path-like semantics.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
