Skip to content

Conversation

@jodersky
Copy link
Member

Process arguments passed to the spark-shell. Fixes running the spark-shell from within a build environment.

Copy link
Member

Choose a reason for hiding this comment

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

Why "-usejavacp" is not added here ?

Copy link
Member Author

Choose a reason for hiding this comment

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

"-usejavacp" is actually passed from the build sparkShell task. I didn't want to change the conditions under which it is called (i.e. it may not be needed if other arguments are supplied)

@vanzin
Copy link
Contributor

vanzin commented Dec 3, 2015

ok to test

@SparkQA
Copy link

SparkQA commented Dec 3, 2015

Test build #47168 has finished for PR 9824 at commit 3ebcf91.

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

@jodersky
Copy link
Member Author

jodersky commented Dec 7, 2015

This PR is pretty low priority but still a neat thing to have when developing.
@vanzin, is this PR acceptable?

@vanzin
Copy link
Contributor

vanzin commented Dec 7, 2015

Code looks ok to me, but this is not an area where I have much familiarity (e.g. I'm not familiar with how the underlying repl works in detail). Perhaps others know? @JoshRosen @andrewor14

@dragos
Copy link
Contributor

dragos commented Dec 9, 2015

@jodersky, I didn't see you PR and opened #10199. I think it makes sense to push yours and close mine, but it would be good to treat errors and stop execution in that case (same way the REPL for 2.10 does). It's only a few lines of code more, as in my PR.

@jodersky
Copy link
Member Author

jodersky commented Dec 9, 2015

+1 for treating the errors. How do you usually deal with overlapping pull requests? Should I just copy your error-handling code manually and mention it?

@dragos
Copy link
Contributor

dragos commented Dec 9, 2015

Copying is fine by me, it's a really small piece of code. I already closed my PR.

@jodersky
Copy link
Member Author

jodersky commented Dec 9, 2015

@dragos, should be good now

@SparkQA
Copy link

SparkQA commented Dec 9, 2015

Test build #47436 has started for PR 9824 at commit 72adf1e.

@dragos
Copy link
Contributor

dragos commented Dec 10, 2015

LGTM. Let's get this in, two PRs in 2 weeks is a good sign this is very useful ;-)

@vanzin
Copy link
Contributor

vanzin commented Dec 10, 2015

Ok, I'm merging this to master.

@asfgit asfgit closed this in db51652 Dec 10, 2015
@jodersky jodersky deleted the shell-2.11 branch December 14, 2015 19:29
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.

5 participants