Skip to content

Conversation

@tnachen
Copy link
Contributor

@tnachen tnachen commented Jul 28, 2014

No description provided.

@SparkQA
Copy link

SparkQA commented Jul 28, 2014

QA tests have started for PR 1622. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17314/consoleFull

@SparkQA
Copy link

SparkQA commented Jul 29, 2014

QA results for PR 1622:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17314/consoleFull

@tnachen
Copy link
Contributor Author

tnachen commented Jul 29, 2014

@kayousterhout I see you're appointed to most of the Mesos/Spark code, wonder if you can take a look at my patch?

@tnachen
Copy link
Contributor Author

tnachen commented Jul 31, 2014

@pwendell I wonder how do I get reviews on patches?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you look at the internals of spark-class, I think you want to be setting SPARK_EXECUTOR_OPTS here. Also, I think environment variables should be set via setEnvironment as shown above, not like this.

@pwendell
Copy link
Contributor

Thanks for looking at this, I proposed a slight change but I think overall this is the right place to be fixing this. One other thing - have you tested this on a Mesos cluster? Thanks!

@pwendell
Copy link
Contributor

pwendell commented Aug 2, 2014

btw - I think this might overlap with #1513

@tnachen
Copy link
Contributor Author

tnachen commented Aug 2, 2014

Thanks @pwendell I think we should use that, but I'm not that familiar with spark yet so please help verify. Thanks!

@andrewor14
Copy link
Contributor

Hi @tnachen, I think this is resolved. Would you mind closing this PR?

@tnachen
Copy link
Contributor Author

tnachen commented Sep 3, 2014

Will do.

@tnachen tnachen closed this Sep 3, 2014
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