Github user tgravescs commented on a diff in the pull request:

    https://github.com/apache/spark/pull/303#discussion_r11399738
  
    --- Diff: 
yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
    @@ -424,29 +408,107 @@ object ClientBase {
         }
       }
     
    -  def populateClasspath(conf: Configuration, sparkConf: SparkConf, 
addLog4j: Boolean, env: HashMap[String, String]) {
    -    Apps.addToEnvironment(env, Environment.CLASSPATH.name, 
Environment.PWD.$())
    -    // If log4j present, ensure ours overrides all others
    -    if (addLog4j) {
    -      Apps.addToEnvironment(env, Environment.CLASSPATH.name, 
Environment.PWD.$() +
    -        Path.SEPARATOR + LOG4J_PROP)
    +  /**
    +   * Returns the java command line argument for setting up log4j. If there 
is a log4j.properties
    +   * in the given local resources, it is used, otherwise the 
SPARK_LOG4J_CONF environment variable
    +   * is checked.
    +   */
    +  def getLog4jConfiguration(localResources: HashMap[String, 
LocalResource]): String = {
    +    var log4jConf = LOG4J_PROP
    +    if (!localResources.contains(log4jConf)) {
    +      log4jConf = System.getenv(LOG4J_CONF_ENV_KEY) match {
    +        case conf: String =>
    +          val confUri = new URI(conf)
    +          if (ClientBase.LOCAL_SCHEME.equals(confUri.getScheme())) {
    +            "file://" + confUri.getPath()
    +          } else {
    +            ClientBase.LOG4J_PROP
    +          }
    +        case null => "log4j-spark-container.properties"
    +      }
    +    }
    +    " -Dlog4j.configuration=" + log4jConf
    +  }
    +
    +  def populateClasspath(args: ClientArguments, conf: Configuration, 
sparkConf: SparkConf,
    +      log4jConf: String, env: HashMap[String, String]) {
    +    if (log4jConf != null) {
    +      // If a custom log4j config file is provided, add its parent 
directory to the
    +      // classpath. Note that for the "local:" case, this only works if 
the custom config's
    +      // file name is "log4j.properties".
    +      val localPath = getLocalPath(log4jConf)
    +      if (localPath != null) {
    +        val parentPath = new File(localPath).getParent()
    +        Apps.addToEnvironment(env, Environment.CLASSPATH.name, parentPath)
    +      } else {
    +        Apps.addToEnvironment(env, Environment.CLASSPATH.name, 
Environment.PWD.$())
    --- End diff --
    
    why aren't we adding the entire path here?  Apps.addToEnvironment(env, 
Environment.CLASSPATH.name, Environment.PWD.$() +   Path.SEPARATOR + LOG4J_PROP)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to