Skip to content

Conversation

@techaddict
Copy link
Contributor

What changes were proposed in this pull request?

Migrate Mesos configs to use ConfigEntry

How was this patch tested?

Jenkins Tests

@SparkQA
Copy link

SparkQA commented Oct 27, 2016

Test build #67614 has finished for PR 15654 at commit af306bd.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@techaddict
Copy link
Contributor Author

cc: @mgummelt @srowen

@mgummelt
Copy link

Thanks! One small fix then LGTM


private val publicAddress = Option(conf.getenv("SPARK_PUBLIC_DNS")).getOrElse(args.host)
private val recoveryMode = conf.get("spark.deploy.recoveryMode", "NONE").toUpperCase()
private val recoveryMode = conf.get(RECOVERY_MODE).getOrElse("NONE").toUpperCase()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the "NONE" default be added to the config builder?

@SparkQA
Copy link

SparkQA commented Oct 31, 2016

Test build #67823 has finished for PR 15654 at commit bb74f52.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@techaddict
Copy link
Contributor Author

@mgummelt done! 👍

@mgummelt
Copy link

cc @srowen for merge into master

@srowen
Copy link
Member

srowen commented Nov 1, 2016

Merged to master

@asfgit asfgit closed this in ec6f479 Nov 1, 2016
@mgummelt
Copy link

mgummelt commented Nov 1, 2016

Thanks @techaddict. Another PR to move the rest of the configs would be very welcome.

@techaddict
Copy link
Contributor Author

@mgummelt yes working on it.

uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
## What changes were proposed in this pull request?

Migrate Mesos configs to use ConfigEntry
## How was this patch tested?

Jenkins Tests

Author: Sandeep Singh <sandeep@techaddict.me>

Closes apache#15654 from techaddict/SPARK-16881.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants