You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mgummelt <gi...@git.apache.org> on 2016/07/13 01:09:06 UTC

[GitHub] spark pull request #14167: [WIP] [SPARK-16194] Mesos Driver env vars

GitHub user mgummelt opened a pull request:

    https://github.com/apache/spark/pull/14167

    [WIP] [SPARK-16194] Mesos Driver env vars

    ## What changes were proposed in this pull request?
    
    Added new configuration namespace: spark.mesos.env.*
    
    This allows a user submitting a job in cluster mode to set arbitrary environment variables on the driver.
    spark.mesos.env.KEY=VAL will result in the env var "KEY" being set to "VAL"
    
    ## How was this patch tested?
    
    unit tests
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mesosphere/spark driver-env-vars

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/14167.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #14167
    
----
commit 4cf039954251f6707e70b201f5eec8b060d43570
Author: Michael Gummelt <mg...@mesosphere.io>
Date:   2016-07-12T22:42:40Z

    refactor

commit e706ca6206ac46cdb304d70c66573a1fcb85060b
Author: Michael Gummelt <mg...@mesosphere.io>
Date:   2016-07-13T01:04:00Z

    tests

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62635/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62341 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62341/consoleFull)** for PR 14167 at commit [`1bfee1d`](https://github.com/apache/spark/commit/1bfee1dd8546eba5c19a8a2fe08a95650de219f1).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70779638
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -353,38 +353,60 @@ private[spark] class MesosClusterScheduler(
         }
       }
     
    -  private def buildDriverCommand(desc: MesosDriverDescription): CommandInfo = {
    -    val appJar = CommandInfo.URI.newBuilder()
    -      .setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
    -    val builder = CommandInfo.newBuilder().addUris(appJar)
    -    val entries = conf.getOption("spark.executor.extraLibraryPath")
    -      .map(path => Seq(path) ++ desc.command.libraryPathEntries)
    -      .getOrElse(desc.command.libraryPathEntries)
    -
    -    val prefixEnv = if (!entries.isEmpty) {
    -      Utils.libraryPathEnvPrefix(entries)
    -    } else {
    -      ""
    +  private def getDriverExecutorURI(desc: MesosDriverDescription) = {
    +    desc.schedulerProperties.get("spark.executor.uri")
    +      .orElse(desc.command.environment.get("SPARK_EXECUTOR_URI"))
    +  }
    +
    +  private def getDriverEnvironment(desc: MesosDriverDescription): Environment = {
    +    val env = {
    +      val executorOpts = desc.schedulerProperties.map { case (k, v) => s"-D$k=$v" }.mkString(" ")
    +      val executorEnv = Map("SPARK_EXECUTOR_OPTS" -> executorOpts)
    +
    +      val prefix = "spark.mesos.env."
    --- End diff --
    
    Spark does not have a  spark.driverEnv.[EnvironmentVariableName] similar to spark.executorEnv.[EnvironmentVariableName] http://spark.apache.org/docs/latest/configuration.html. 
    From a UX experience and name consistency view i would expect something like that. The problem is that this pr only handles the mesos case so we cannot rename it to that name (unless we explicitly defined it in docs), also in client mode you do not need that so you will need to ignore it.
    "spark.mesos.env." needs to be more specific like spark.mesos.driver.env but as i said it only makes sense in cluster mode and  spark.driverEnv seems more appropriate.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [WIP] [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70784310
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -399,20 +421,20 @@ private[spark] class MesosClusterScheduler(
           // Sandbox points to the current directory by default with Mesos.
           (cmdExecutable, ".")
         }
    -    val primaryResource = new File(sandboxPath, desc.jarUrl.split("/").last).toString()
         val cmdOptions = generateCmdOption(desc, sandboxPath).mkString(" ")
    +    val primaryResource = new File(sandboxPath, desc.jarUrl.split("/").last).toString()
         val appArguments = desc.command.arguments.mkString(" ")
    -    builder.setValue(s"$executable $cmdOptions $primaryResource $appArguments")
    -    builder.setEnvironment(envBuilder.build())
    -    conf.getOption("spark.mesos.uris").map { uris =>
    -      setupUris(uris, builder)
    -    }
    -    desc.schedulerProperties.get("spark.mesos.uris").map { uris =>
    -      setupUris(uris, builder)
    -    }
    -    desc.schedulerProperties.get("spark.submit.pyFiles").map { pyFiles =>
    -      setupUris(pyFiles, builder)
    -    }
    +
    +    s"$executable $cmdOptions $primaryResource $appArguments"
    +  }
    +
    +  private def buildDriverCommand(desc: MesosDriverDescription): CommandInfo = {
    +    val builder = CommandInfo.newBuilder()
    +
    +    builder.setValue(getDriverCommandValue(desc))
    +    builder.setEnvironment(getDriverEnvironment(desc))
    +    builder.addAllUris(getDriverUris(desc).asJava)
    +
    --- End diff --
    
    We should be consistent i think with spaces. In other methods i see no space between return argument and the rest of the  method body ( vs https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackend.scala#L422). 
    
    The same applies for the spaces between variable declaration and statements ( vs https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackend.scala#L142) .


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [WIP] [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62201 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62201/consoleFull)** for PR 14167 at commit [`e706ca6`](https://github.com/apache/spark/commit/e706ca6206ac46cdb304d70c66573a1fcb85060b).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70955202
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/Utils.scala ---
    @@ -0,0 +1,72 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster.mesos
    +
    +import java.util.Collections
    +
    +import org.apache.mesos.Protos._
    +import org.apache.mesos.Protos.Value.Scalar
    +import org.apache.mesos.SchedulerDriver
    +import org.mockito.{ArgumentCaptor, Matchers}
    +import org.mockito.Mockito._
    +import scala.collection.JavaConverters._
    +
    +object Utils {
    +  def createOffer(offerId: String, slaveId: String, mem: Int, cpu: Int): Offer = {
    +    val builder = Offer.newBuilder()
    +    builder.addResourcesBuilder()
    +      .setName("mem")
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Scalar.newBuilder().setValue(mem))
    +    builder.addResourcesBuilder()
    +      .setName("cpus")
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Scalar.newBuilder().setValue(cpu))
    +    builder.setId(createOfferId(offerId))
    +      .setFrameworkId(FrameworkID.newBuilder()
    +        .setValue("f1"))
    +      .setSlaveId(SlaveID.newBuilder().setValue(slaveId))
    +      .setHostname(s"host${slaveId}")
    --- End diff --
    
    {} redundant


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r71671756
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/Utils.scala ---
    @@ -0,0 +1,72 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster.mesos
    +
    +import java.util.Collections
    +
    +import org.apache.mesos.Protos._
    +import org.apache.mesos.Protos.Value.Scalar
    +import org.apache.mesos.SchedulerDriver
    +import org.mockito.{ArgumentCaptor, Matchers}
    +import org.mockito.Mockito._
    +import scala.collection.JavaConverters._
    +
    +object Utils {
    +  def createOffer(offerId: String, slaveId: String, mem: Int, cpu: Int): Offer = {
    +    val builder = Offer.newBuilder()
    +    builder.addResourcesBuilder()
    +      .setName("mem")
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Scalar.newBuilder().setValue(mem))
    +    builder.addResourcesBuilder()
    +      .setName("cpus")
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Scalar.newBuilder().setValue(cpu))
    +    builder.setId(createOfferId(offerId))
    +      .setFrameworkId(FrameworkID.newBuilder()
    +        .setValue("f1"))
    +      .setSlaveId(SlaveID.newBuilder().setValue(slaveId))
    +      .setHostname(s"host${slaveId}")
    --- End diff --
    
    same


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [WIP] [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r71047287
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterSchedulerSuite.scala ---
    @@ -172,4 +187,28 @@ class MesosClusterSchedulerSuite extends SparkFunSuite with LocalSparkContext wi
           assert(escape(s"onlywrap${char}this") === wrapped(s"onlywrap${char}this"))
         })
       }
    +
    +  test("supports spark.mesos.driverEnv.*") {
    +    setScheduler()
    +
    +    val mem = 1000
    +    val cpu = 1
    +
    +    val response = scheduler.submitDriver(
    +      new MesosDriverDescription("d1", "jar", mem, cpu, true,
    +        command,
    +        Map("spark.mesos.executor.home" -> "test",
    +          "spark.app.name" -> "test",
    +          "spark.mesos.driverEnv.TEST_ENV" -> "TEST_VAL"),
    +        "s1",
    +        new Date()))
    +    assert(response.success)
    +
    +    val offer = Utils.createOffer("o1", "s1", mem, cpu)
    +    scheduler.resourceOffers(driver, List(offer).asJava)
    +    val tasks = Utils.verifyTaskLaunched(driver, "o1")
    +    val env = tasks(0).getCommand.getEnvironment.getVariablesList.asScala.map(v =>
    --- End diff --
    
    done


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r71047405
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/Utils.scala ---
    @@ -0,0 +1,72 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster.mesos
    +
    +import java.util.Collections
    +
    +import org.apache.mesos.Protos._
    +import org.apache.mesos.Protos.Value.Scalar
    +import org.apache.mesos.SchedulerDriver
    +import org.mockito.{ArgumentCaptor, Matchers}
    +import org.mockito.Mockito._
    +import scala.collection.JavaConverters._
    +
    +object Utils {
    +  def createOffer(offerId: String, slaveId: String, mem: Int, cpu: Int): Offer = {
    +    val builder = Offer.newBuilder()
    +    builder.addResourcesBuilder()
    +      .setName("mem")
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Scalar.newBuilder().setValue(mem))
    +    builder.addResourcesBuilder()
    +      .setName("cpus")
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Scalar.newBuilder().setValue(cpu))
    +    builder.setId(createOfferId(offerId))
    +      .setFrameworkId(FrameworkID.newBuilder()
    +        .setValue("f1"))
    +      .setSlaveId(SlaveID.newBuilder().setValue(slaveId))
    +      .setHostname(s"host${slaveId}")
    +      .build()
    +  }
    +
    +  def verifyTaskLaunched(driver: SchedulerDriver,
    +                         offerId: String): List[TaskInfo] = {
    --- End diff --
    
    fixed


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70851528
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -353,38 +353,60 @@ private[spark] class MesosClusterScheduler(
         }
       }
     
    -  private def buildDriverCommand(desc: MesosDriverDescription): CommandInfo = {
    -    val appJar = CommandInfo.URI.newBuilder()
    -      .setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
    -    val builder = CommandInfo.newBuilder().addUris(appJar)
    -    val entries = conf.getOption("spark.executor.extraLibraryPath")
    -      .map(path => Seq(path) ++ desc.command.libraryPathEntries)
    -      .getOrElse(desc.command.libraryPathEntries)
    -
    -    val prefixEnv = if (!entries.isEmpty) {
    -      Utils.libraryPathEnvPrefix(entries)
    -    } else {
    -      ""
    +  private def getDriverExecutorURI(desc: MesosDriverDescription) = {
    +    desc.schedulerProperties.get("spark.executor.uri")
    +      .orElse(desc.command.environment.get("SPARK_EXECUTOR_URI"))
    +  }
    +
    +  private def getDriverEnvironment(desc: MesosDriverDescription): Environment = {
    +    val env = {
    +      val executorOpts = desc.schedulerProperties.map { case (k, v) => s"-D$k=$v" }.mkString(" ")
    +      val executorEnv = Map("SPARK_EXECUTOR_OPTS" -> executorOpts)
    +
    +      val prefix = "spark.mesos.env."
    --- End diff --
    
    > From a UX experience and name consistency view i would expect something like that.
    
    Yea I went back and forth on this.  I'm fine with spark.mesos.driverEnv  Does that work?
    
    > also in client mode you do not need that so you will need to ignore it.
    
    Do you mean there needs to be some code change to ignore it?  The only handling code is in the cluster dispatcher, so it's effectively ignored in client mode.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/14167


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    LGTM other than minor style issues.  I run our tests against it so refactoring is successful i guess.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70893281
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/Utils.scala ---
    @@ -0,0 +1,72 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster.mesos
    +
    +import java.util.Collections
    +
    +import org.apache.mesos.Protos._
    +import org.apache.mesos.Protos.Value.Scalar
    +import org.apache.mesos.SchedulerDriver
    +import org.mockito.{ArgumentCaptor, Matchers}
    +import org.mockito.Mockito._
    +import scala.collection.JavaConverters._
    +
    +object Utils {
    +  def createOffer(offerId: String, slaveId: String, mem: Int, cpu: Int): Offer = {
    +    val builder = Offer.newBuilder()
    +    builder.addResourcesBuilder()
    +      .setName("mem")
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Scalar.newBuilder().setValue(mem))
    +    builder.addResourcesBuilder()
    +      .setName("cpus")
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Scalar.newBuilder().setValue(cpu))
    +    builder.setId(createOfferId(offerId))
    +      .setFrameworkId(FrameworkID.newBuilder()
    +        .setValue("f1"))
    +      .setSlaveId(SlaveID.newBuilder().setValue(slaveId))
    +      .setHostname(s"host${slaveId}")
    +      .build()
    +  }
    +
    +  def verifyTaskLaunched(driver: SchedulerDriver,
    +                         offerId: String): List[TaskInfo] = {
    --- End diff --
    
    Pls fiix indentation for method parameters (4 spaces, first parameter in a new line). 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62629/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [WIP] [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62270 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62270/consoleFull)** for PR 14167 at commit [`37dc48a`](https://github.com/apache/spark/commit/37dc48acd35b674144eebf07f0478fce739b48b5).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62340/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62391 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62391/consoleFull)** for PR 14167 at commit [`de75386`](https://github.com/apache/spark/commit/de7538616e63d8a80aac0814a1c7133989e61817).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r71047393
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/Utils.scala ---
    @@ -0,0 +1,72 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.scheduler.cluster.mesos
    +
    +import java.util.Collections
    +
    +import org.apache.mesos.Protos._
    +import org.apache.mesos.Protos.Value.Scalar
    +import org.apache.mesos.SchedulerDriver
    +import org.mockito.{ArgumentCaptor, Matchers}
    +import org.mockito.Mockito._
    +import scala.collection.JavaConverters._
    +
    +object Utils {
    +  def createOffer(offerId: String, slaveId: String, mem: Int, cpu: Int): Offer = {
    +    val builder = Offer.newBuilder()
    +    builder.addResourcesBuilder()
    +      .setName("mem")
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Scalar.newBuilder().setValue(mem))
    +    builder.addResourcesBuilder()
    +      .setName("cpus")
    +      .setType(Value.Type.SCALAR)
    +      .setScalar(Scalar.newBuilder().setValue(cpu))
    +    builder.setId(createOfferId(offerId))
    +      .setFrameworkId(FrameworkID.newBuilder()
    +        .setValue("f1"))
    +      .setSlaveId(SlaveID.newBuilder().setValue(slaveId))
    +      .setHostname(s"host${slaveId}")
    --- End diff --
    
    This also isn't a change from this PR.  Though generally I do like to encase all interpolated expressions in braces just so the parsing is unambiguous 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    I don't know anything about mesos, but I presume y'all are pretty expert. Unless anyone else with Mesos knowledge chimes in, and tests pass and all that, OK to merge.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62629 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62629/consoleFull)** for PR 14167 at commit [`b9c396b`](https://github.com/apache/spark/commit/b9c396b4bfd01313af9f728035e1093c66a9016a).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    @skonto comments addressed.  Please re-review.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    @srowen Can we get a merge?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62391 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62391/consoleFull)** for PR 14167 at commit [`de75386`](https://github.com/apache/spark/commit/de7538616e63d8a80aac0814a1c7133989e61817).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62341/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62340 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62340/consoleFull)** for PR 14167 at commit [`9f7b151`](https://github.com/apache/spark/commit/9f7b15198f8834334cac09b0a76cda649081bbd2).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [WIP] [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62201 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62201/consoleFull)** for PR 14167 at commit [`e706ca6`](https://github.com/apache/spark/commit/e706ca6206ac46cdb304d70c66573a1fcb85060b).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    great thnx @srowen 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62635 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62635/consoleFull)** for PR 14167 at commit [`1824d63`](https://github.com/apache/spark/commit/1824d63c04b63c8392708746bd22f4a8662b5e78).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    The build fails with: [error] /home/jenkins/workspace/SparkPullRequestBuilder@4/core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterSchedulerSuite.scala:34: object Utils is not a member of package org.apache.spark.scheduler.cluster.mesos
    [error] import org.apache.spark.scheduler.cluster.mesos.Utils._
    
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62270/consoleFull
    
    I reproduce it locally you are missing the Utils object.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    LGTM. I agree with @viirya for the description.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    @skonto fixed the style issues
    
    @andrewor14 plz merge


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    @andrewor14 @srowen pls merge


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70954615
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -353,38 +353,60 @@ private[spark] class MesosClusterScheduler(
         }
       }
     
    -  private def buildDriverCommand(desc: MesosDriverDescription): CommandInfo = {
    -    val appJar = CommandInfo.URI.newBuilder()
    -      .setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
    -    val builder = CommandInfo.newBuilder().addUris(appJar)
    -    val entries = conf.getOption("spark.executor.extraLibraryPath")
    -      .map(path => Seq(path) ++ desc.command.libraryPathEntries)
    -      .getOrElse(desc.command.libraryPathEntries)
    -
    -    val prefixEnv = if (!entries.isEmpty) {
    -      Utils.libraryPathEnvPrefix(entries)
    -    } else {
    -      ""
    +  private def getDriverExecutorURI(desc: MesosDriverDescription) = {
    +    desc.schedulerProperties.get("spark.executor.uri")
    +      .orElse(desc.command.environment.get("SPARK_EXECUTOR_URI"))
    +  }
    +
    +  private def getDriverEnvironment(desc: MesosDriverDescription): Environment = {
    +    val env = {
    +      val executorOpts = desc.schedulerProperties.map { case (k, v) => s"-D$k=$v" }.mkString(" ")
    +      val executorEnv = Map("SPARK_EXECUTOR_OPTS" -> executorOpts)
    +
    +      val prefix = "spark.mesos.driverEnv."
    +      val driverEnv = desc.schedulerProperties.filterKeys(_.startsWith(prefix))
    +        .map { case (k, v) => (k.substring(prefix.length), v) }
    +
    +      driverEnv ++ executorEnv ++ desc.command.environment
         }
    +
         val envBuilder = Environment.newBuilder()
    -    desc.command.environment.foreach { case (k, v) =>
    -      envBuilder.addVariables(Variable.newBuilder().setName(k).setValue(v).build())
    +    env.foreach { case (k, v) =>
    +      envBuilder.addVariables(Variable.newBuilder().setName(k).setValue(v))
         }
    -    // Pass all spark properties to executor.
    -    val executorOpts = desc.schedulerProperties.map { case (k, v) => s"-D$k=$v" }.mkString(" ")
    -    envBuilder.addVariables(
    -      Variable.newBuilder().setName("SPARK_EXECUTOR_OPTS").setValue(executorOpts))
    +    envBuilder.build()
    +  }
    +
    +  private def getDriverUris(desc: MesosDriverDescription): List[CommandInfo.URI] = {
    +    val confUris = List(conf.getOption("spark.mesos.uris"),
    +      desc.schedulerProperties.get("spark.mesos.uris"),
    +      desc.schedulerProperties.get("spark.submit.pyFiles")).flatMap(
    +      _.map(_.split(",").map(_.trim))
    +    ).flatten
    +
    +    val jarUrl = desc.jarUrl.stripPrefix("file:").stripPrefix("local:")
    +
    +    ((jarUrl :: confUris) ++ getDriverExecutorURI(desc).toList).map(uri =>
    +      CommandInfo.URI.newBuilder().setValue(uri.trim()).build())
    +  }
    +
    +  private def getDriverCommandValue(desc: MesosDriverDescription): String = {
         val dockerDefined = desc.schedulerProperties.contains("spark.mesos.executor.docker.image")
    -    val executorUri = desc.schedulerProperties.get("spark.executor.uri")
    -      .orElse(desc.command.environment.get("SPARK_EXECUTOR_URI"))
    +    val executorUri = getDriverExecutorURI(desc)
         // Gets the path to run spark-submit, and the path to the Mesos sandbox.
         val (executable, sandboxPath) = if (dockerDefined) {
           // Application jar is automatically downloaded in the mounted sandbox by Mesos,
           // and the path to the mounted volume is stored in $MESOS_SANDBOX env variable.
           ("./bin/spark-submit", "$MESOS_SANDBOX")
         } else if (executorUri.isDefined) {
    -      builder.addUris(CommandInfo.URI.newBuilder().setValue(executorUri.get).build())
           val folderBasename = executorUri.get.split('/').last.split('.').head
    +
    +      val entries = conf.getOption("spark.executor.extraLibraryPath")
    +        .map(path => Seq(path) ++ desc.command.libraryPathEntries)
    +        .getOrElse(desc.command.libraryPathEntries)
    +
    +      val prefixEnv = if (!entries.isEmpty) Utils.libraryPathEnvPrefix(entries) else ""
    --- End diff --
    
    Replace with entries.nonEmpty


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r71047091
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -399,20 +421,18 @@ private[spark] class MesosClusterScheduler(
           // Sandbox points to the current directory by default with Mesos.
           (cmdExecutable, ".")
         }
    -    val primaryResource = new File(sandboxPath, desc.jarUrl.split("/").last).toString()
         val cmdOptions = generateCmdOption(desc, sandboxPath).mkString(" ")
    +    val primaryResource = new File(sandboxPath, desc.jarUrl.split("/").last).toString()
    --- End diff --
    
    Same here.  Unrelated to this PR.  


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62341 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62341/consoleFull)** for PR 14167 at commit [`1bfee1d`](https://github.com/apache/spark/commit/1bfee1dd8546eba5c19a8a2fe08a95650de219f1).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70851763
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -399,20 +421,20 @@ private[spark] class MesosClusterScheduler(
           // Sandbox points to the current directory by default with Mesos.
           (cmdExecutable, ".")
         }
    -    val primaryResource = new File(sandboxPath, desc.jarUrl.split("/").last).toString()
         val cmdOptions = generateCmdOption(desc, sandboxPath).mkString(" ")
    +    val primaryResource = new File(sandboxPath, desc.jarUrl.split("/").last).toString()
         val appArguments = desc.command.arguments.mkString(" ")
    -    builder.setValue(s"$executable $cmdOptions $primaryResource $appArguments")
    -    builder.setEnvironment(envBuilder.build())
    -    conf.getOption("spark.mesos.uris").map { uris =>
    -      setupUris(uris, builder)
    -    }
    -    desc.schedulerProperties.get("spark.mesos.uris").map { uris =>
    -      setupUris(uris, builder)
    -    }
    -    desc.schedulerProperties.get("spark.submit.pyFiles").map { pyFiles =>
    -      setupUris(pyFiles, builder)
    -    }
    +
    +    s"$executable $cmdOptions $primaryResource $appArguments"
    +  }
    +
    +  private def buildDriverCommand(desc: MesosDriverDescription): CommandInfo = {
    +    val builder = CommandInfo.newBuilder()
    +
    +    builder.setValue(getDriverCommandValue(desc))
    +    builder.setEnvironment(getDriverEnvironment(desc))
    +    builder.addAllUris(getDriverUris(desc).asJava)
    +
    --- End diff --
    
    removed the newlines


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Error message from build: `ERROR: Step ?Publish JUnit test result report? failed: No test report files were found. Configuration error?`
    



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Note that I'll add docs once the code is :+1: 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62391/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    nit: From the code, it is `spark.mesos.driverEnv` but in PR description it is `spark.mesos.env`.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [WIP] [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62270 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62270/consoleFull)** for PR 14167 at commit [`37dc48a`](https://github.com/apache/spark/commit/37dc48acd35b674144eebf07f0478fce739b48b5).
     * This patch **fails to build**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62635 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62635/consoleFull)** for PR 14167 at commit [`1824d63`](https://github.com/apache/spark/commit/1824d63c04b63c8392708746bd22f4a8662b5e78).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r71671714
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -399,20 +421,18 @@ private[spark] class MesosClusterScheduler(
           // Sandbox points to the current directory by default with Mesos.
           (cmdExecutable, ".")
         }
    -    val primaryResource = new File(sandboxPath, desc.jarUrl.split("/").last).toString()
         val cmdOptions = generateCmdOption(desc, sandboxPath).mkString(" ")
    +    val primaryResource = new File(sandboxPath, desc.jarUrl.split("/").last).toString()
    --- End diff --
    
    same


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70954807
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -399,20 +421,18 @@ private[spark] class MesosClusterScheduler(
           // Sandbox points to the current directory by default with Mesos.
           (cmdExecutable, ".")
         }
    -    val primaryResource = new File(sandboxPath, desc.jarUrl.split("/").last).toString()
         val cmdOptions = generateCmdOption(desc, sandboxPath).mkString(" ")
    +    val primaryResource = new File(sandboxPath, desc.jarUrl.split("/").last).toString()
    --- End diff --
    
    no need for parentheses use: toString


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r71671684
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -353,38 +353,60 @@ private[spark] class MesosClusterScheduler(
         }
       }
     
    -  private def buildDriverCommand(desc: MesosDriverDescription): CommandInfo = {
    -    val appJar = CommandInfo.URI.newBuilder()
    -      .setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
    -    val builder = CommandInfo.newBuilder().addUris(appJar)
    -    val entries = conf.getOption("spark.executor.extraLibraryPath")
    -      .map(path => Seq(path) ++ desc.command.libraryPathEntries)
    -      .getOrElse(desc.command.libraryPathEntries)
    -
    -    val prefixEnv = if (!entries.isEmpty) {
    -      Utils.libraryPathEnvPrefix(entries)
    -    } else {
    -      ""
    +  private def getDriverExecutorURI(desc: MesosDriverDescription) = {
    +    desc.schedulerProperties.get("spark.executor.uri")
    +      .orElse(desc.command.environment.get("SPARK_EXECUTOR_URI"))
    +  }
    +
    +  private def getDriverEnvironment(desc: MesosDriverDescription): Environment = {
    +    val env = {
    +      val executorOpts = desc.schedulerProperties.map { case (k, v) => s"-D$k=$v" }.mkString(" ")
    +      val executorEnv = Map("SPARK_EXECUTOR_OPTS" -> executorOpts)
    +
    +      val prefix = "spark.mesos.driverEnv."
    +      val driverEnv = desc.schedulerProperties.filterKeys(_.startsWith(prefix))
    +        .map { case (k, v) => (k.substring(prefix.length), v) }
    +
    +      driverEnv ++ executorEnv ++ desc.command.environment
         }
    +
         val envBuilder = Environment.newBuilder()
    -    desc.command.environment.foreach { case (k, v) =>
    -      envBuilder.addVariables(Variable.newBuilder().setName(k).setValue(v).build())
    +    env.foreach { case (k, v) =>
    +      envBuilder.addVariables(Variable.newBuilder().setName(k).setValue(v))
         }
    -    // Pass all spark properties to executor.
    -    val executorOpts = desc.schedulerProperties.map { case (k, v) => s"-D$k=$v" }.mkString(" ")
    -    envBuilder.addVariables(
    -      Variable.newBuilder().setName("SPARK_EXECUTOR_OPTS").setValue(executorOpts))
    +    envBuilder.build()
    +  }
    +
    +  private def getDriverUris(desc: MesosDriverDescription): List[CommandInfo.URI] = {
    +    val confUris = List(conf.getOption("spark.mesos.uris"),
    +      desc.schedulerProperties.get("spark.mesos.uris"),
    +      desc.schedulerProperties.get("spark.submit.pyFiles")).flatMap(
    +      _.map(_.split(",").map(_.trim))
    +    ).flatten
    +
    +    val jarUrl = desc.jarUrl.stripPrefix("file:").stripPrefix("local:")
    +
    +    ((jarUrl :: confUris) ++ getDriverExecutorURI(desc).toList).map(uri =>
    +      CommandInfo.URI.newBuilder().setValue(uri.trim()).build())
    +  }
    +
    +  private def getDriverCommandValue(desc: MesosDriverDescription): String = {
         val dockerDefined = desc.schedulerProperties.contains("spark.mesos.executor.docker.image")
    -    val executorUri = desc.schedulerProperties.get("spark.executor.uri")
    -      .orElse(desc.command.environment.get("SPARK_EXECUTOR_URI"))
    +    val executorUri = getDriverExecutorURI(desc)
         // Gets the path to run spark-submit, and the path to the Mesos sandbox.
         val (executable, sandboxPath) = if (dockerDefined) {
           // Application jar is automatically downloaded in the mounted sandbox by Mesos,
           // and the path to the mounted volume is stored in $MESOS_SANDBOX env variable.
           ("./bin/spark-submit", "$MESOS_SANDBOX")
         } else if (executorUri.isDefined) {
    -      builder.addUris(CommandInfo.URI.newBuilder().setValue(executorUri.get).build())
           val folderBasename = executorUri.get.split('/').last.split('.').head
    +
    +      val entries = conf.getOption("spark.executor.extraLibraryPath")
    +        .map(path => Seq(path) ++ desc.command.libraryPathEntries)
    +        .getOrElse(desc.command.libraryPathEntries)
    +
    +      val prefixEnv = if (!entries.isEmpty) Utils.libraryPathEnvPrefix(entries) else ""
    --- End diff --
    
    ok for future fix then


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62340 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62340/consoleFull)** for PR 14167 at commit [`9f7b151`](https://github.com/apache/spark/commit/9f7b15198f8834334cac09b0a76cda649081bbd2).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70781247
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackendSuite.scala ---
    @@ -35,6 +34,7 @@ import org.apache.spark.{LocalSparkContext, SecurityManager, SparkConf, SparkCon
     import org.apache.spark.network.shuffle.mesos.MesosExternalShuffleClient
     import org.apache.spark.rpc.RpcEndpointRef
     import org.apache.spark.scheduler.TaskSchedulerImpl
    +import org.apache.spark.scheduler.cluster.mesos.Utils._
    --- End diff --
    
    Wild card imports should be avoided (unless you are importing more than 6 entities, or implicit methods) https://github.com/databricks/scala-style-guide. 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [WIP] [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62201/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [WIP] [SPARK-16194] Mesos Driver env vars

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62270/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Merged to master


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    Updated the description and added docs.  


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70852173
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosCoarseGrainedSchedulerBackendSuite.scala ---
    @@ -35,6 +34,7 @@ import org.apache.spark.{LocalSparkContext, SecurityManager, SparkConf, SparkCon
     import org.apache.spark.network.shuffle.mesos.MesosExternalShuffleClient
     import org.apache.spark.rpc.RpcEndpointRef
     import org.apache.spark.scheduler.TaskSchedulerImpl
    +import org.apache.spark.scheduler.cluster.mesos.Utils._
    --- End diff --
    
    Removed the wildcard.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by mgummelt <gi...@git.apache.org>.
Github user mgummelt commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r71046997
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala ---
    @@ -353,38 +353,60 @@ private[spark] class MesosClusterScheduler(
         }
       }
     
    -  private def buildDriverCommand(desc: MesosDriverDescription): CommandInfo = {
    -    val appJar = CommandInfo.URI.newBuilder()
    -      .setValue(desc.jarUrl.stripPrefix("file:").stripPrefix("local:")).build()
    -    val builder = CommandInfo.newBuilder().addUris(appJar)
    -    val entries = conf.getOption("spark.executor.extraLibraryPath")
    -      .map(path => Seq(path) ++ desc.command.libraryPathEntries)
    -      .getOrElse(desc.command.libraryPathEntries)
    -
    -    val prefixEnv = if (!entries.isEmpty) {
    -      Utils.libraryPathEnvPrefix(entries)
    -    } else {
    -      ""
    +  private def getDriverExecutorURI(desc: MesosDriverDescription) = {
    +    desc.schedulerProperties.get("spark.executor.uri")
    +      .orElse(desc.command.environment.get("SPARK_EXECUTOR_URI"))
    +  }
    +
    +  private def getDriverEnvironment(desc: MesosDriverDescription): Environment = {
    +    val env = {
    +      val executorOpts = desc.schedulerProperties.map { case (k, v) => s"-D$k=$v" }.mkString(" ")
    +      val executorEnv = Map("SPARK_EXECUTOR_OPTS" -> executorOpts)
    +
    +      val prefix = "spark.mesos.driverEnv."
    +      val driverEnv = desc.schedulerProperties.filterKeys(_.startsWith(prefix))
    +        .map { case (k, v) => (k.substring(prefix.length), v) }
    +
    +      driverEnv ++ executorEnv ++ desc.command.environment
         }
    +
         val envBuilder = Environment.newBuilder()
    -    desc.command.environment.foreach { case (k, v) =>
    -      envBuilder.addVariables(Variable.newBuilder().setName(k).setValue(v).build())
    +    env.foreach { case (k, v) =>
    +      envBuilder.addVariables(Variable.newBuilder().setName(k).setValue(v))
         }
    -    // Pass all spark properties to executor.
    -    val executorOpts = desc.schedulerProperties.map { case (k, v) => s"-D$k=$v" }.mkString(" ")
    -    envBuilder.addVariables(
    -      Variable.newBuilder().setName("SPARK_EXECUTOR_OPTS").setValue(executorOpts))
    +    envBuilder.build()
    +  }
    +
    +  private def getDriverUris(desc: MesosDriverDescription): List[CommandInfo.URI] = {
    +    val confUris = List(conf.getOption("spark.mesos.uris"),
    +      desc.schedulerProperties.get("spark.mesos.uris"),
    +      desc.schedulerProperties.get("spark.submit.pyFiles")).flatMap(
    +      _.map(_.split(",").map(_.trim))
    +    ).flatten
    +
    +    val jarUrl = desc.jarUrl.stripPrefix("file:").stripPrefix("local:")
    +
    +    ((jarUrl :: confUris) ++ getDriverExecutorURI(desc).toList).map(uri =>
    +      CommandInfo.URI.newBuilder().setValue(uri.trim()).build())
    +  }
    +
    +  private def getDriverCommandValue(desc: MesosDriverDescription): String = {
         val dockerDefined = desc.schedulerProperties.contains("spark.mesos.executor.docker.image")
    -    val executorUri = desc.schedulerProperties.get("spark.executor.uri")
    -      .orElse(desc.command.environment.get("SPARK_EXECUTOR_URI"))
    +    val executorUri = getDriverExecutorURI(desc)
         // Gets the path to run spark-submit, and the path to the Mesos sandbox.
         val (executable, sandboxPath) = if (dockerDefined) {
           // Application jar is automatically downloaded in the mounted sandbox by Mesos,
           // and the path to the mounted volume is stored in $MESOS_SANDBOX env variable.
           ("./bin/spark-submit", "$MESOS_SANDBOX")
         } else if (executorUri.isDefined) {
    -      builder.addUris(CommandInfo.URI.newBuilder().setValue(executorUri.get).build())
           val folderBasename = executorUri.get.split('/').last.split('.').head
    +
    +      val entries = conf.getOption("spark.executor.extraLibraryPath")
    +        .map(path => Seq(path) ++ desc.command.libraryPathEntries)
    +        .getOrElse(desc.command.libraryPathEntries)
    +
    +      val prefixEnv = if (!entries.isEmpty) Utils.libraryPathEnvPrefix(entries) else ""
    --- End diff --
    
    This is existing code that I didn't change.  I just moved some things around.  I'd prefer not to change code not related to this PR.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #14167: [SPARK-16194] Mesos Driver env vars

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14167
  
    **[Test build #62629 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62629/consoleFull)** for PR 14167 at commit [`b9c396b`](https://github.com/apache/spark/commit/b9c396b4bfd01313af9f728035e1093c66a9016a).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #14167: [SPARK-16194] Mesos Driver env vars

Posted by skonto <gi...@git.apache.org>.
Github user skonto commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14167#discussion_r70955348
  
    --- Diff: core/src/test/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterSchedulerSuite.scala ---
    @@ -172,4 +187,28 @@ class MesosClusterSchedulerSuite extends SparkFunSuite with LocalSparkContext wi
           assert(escape(s"onlywrap${char}this") === wrapped(s"onlywrap${char}this"))
         })
       }
    +
    +  test("supports spark.mesos.driverEnv.*") {
    +    setScheduler()
    +
    +    val mem = 1000
    +    val cpu = 1
    +
    +    val response = scheduler.submitDriver(
    +      new MesosDriverDescription("d1", "jar", mem, cpu, true,
    +        command,
    +        Map("spark.mesos.executor.home" -> "test",
    +          "spark.app.name" -> "test",
    +          "spark.mesos.driverEnv.TEST_ENV" -> "TEST_VAL"),
    +        "s1",
    +        new Date()))
    +    assert(response.success)
    +
    +    val offer = Utils.createOffer("o1", "s1", mem, cpu)
    +    scheduler.resourceOffers(driver, List(offer).asJava)
    +    val tasks = Utils.verifyTaskLaunched(driver, "o1")
    +    val env = tasks(0).getCommand.getEnvironment.getVariablesList.asScala.map(v =>
    --- End diff --
    
    use head instead of task(0)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org