You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mateiz <gi...@git.apache.org> on 2014/04/02 05:05:10 UTC
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
GitHub user mateiz opened a pull request:
https://github.com/apache/spark/pull/294
[SPARK-1134] Fix and document passing of arguments to IPython
This is based on @dianacaroll's previous pull request https://github.com/apache/spark/pull/227, and @joshrosen's comments on https://github.com/apache/spark/pull/38. Since we do want to allow passing arguments to IPython, this does the following:
* It documents that IPython can't be used with standalone jobs for now. (Later versions of IPython will deal with PYTHONSTARTUP properly and enable this, see https://github.com/ipython/ipython/pull/5226, but no released version has that fix.)
* If you run `pyspark` with `IPYTHON=1`, it passes your command-line arguments to it. This way you can do stuff like `IPYTHON=1 bin/pyspark notebook`.
* The old `IPYTHON_OPTS` remains, but I've removed it from the documentation. This is in case people read an old tutorial that uses it.
This is not a perfect solution and I'd also be okay with keeping things as they are today (ignoring `$@` for IPython and using IPYTHON_OPTS), and only doing the doc change. With this change though, when IPython fixes https://github.com/ipython/ipython/pull/5226, people will immediately be able to do `IPYTHON=1 bin/pyspark myscript.py` to run a standalone script and get all the benefits of running scripts in IPython (presumably better debugging and such). Without it, there will be no way to run scripts in IPython.
@joshrosen you should probably take the final call on this.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mateiz/spark spark-1134
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/294.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 #294
----
commit 747bb1347e09d4bd6c86d24d452b569b9071622f
Author: Diana Carroll <dc...@cloudera.com>
Date: 2014-02-27T22:39:43Z
SPARK-1134 bug with ipython prevents non-interactive use with spark; only call ipython if no command line arguments were supplied
commit 87de70ba0a5b804059f02b2f0ac4af55743b95b2
Author: Matei Zaharia <ma...@databricks.com>
Date: 2014-04-02T02:53:50Z
Pass command-line args to IPython, and update docs to describe this
commit cd2036bff469ca048837bed13ae6f81a9c8d1c5a
Author: Matei Zaharia <ma...@databricks.com>
Date: 2014-04-02T02:59:36Z
Doc tweak
----
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by dianacarroll <gi...@git.apache.org>.
Github user dianacarroll commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39340552
Well, my perspective is always that of a new easily confused user. (That's
my target audience.) I myself got tired of typing "IPYTHON=1" every time I
started pyspark, so I did what I'm guessing most people will do which is to
set that as an environment variable in my profile. Which was fine until
the first time I tried running a pyspark script.
Your doc change explicitly recommends I not do that, but...well, really?
It just makes learning pyspark that much more confusing. The "pyspark"
command is going to be most new users' main entry point into this new
technology.
In the new Spark class I'm working on, which uses mainly Python, I had
planned all along to set IPYTHON for the students automatically, because it
is sooooo much easier working in IPython than vanilla, and so tedious to
have to type that in repeatedly (or use command history every time.)
I think what's going to happen is that users will ignore your admonishment
to explicitly type the variable setting every time they start the shell,
and set it in their environment...then they will end up scratching their
heads trying to figure out why their scripts aren't working. (The error
that results is quite non-intuitive for a Spark newbie.)
I can live with it as is (with the doc change) but it isn't a very user
friendly thing.
On Wed, Apr 2, 2014 at 12:31 AM, UCB AMPLab <no...@github.com>wrote:
> Merged build finished. All automated tests passed.
>
> --
> Reply to this email directly or view it on GitHub<https://github.com/apache/spark/pull/294#issuecomment-39287958>
> .
>
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39395517
Merged build finished. All automated tests 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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39389710
Merged build started.
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39514632
@dianacarroll I've merged this in now, using just your original commit (https://github.com/mateiz/spark/commit/747bb1347e09d4bd6c86d24d452b569b9071622f). I think that's the best solution for now. Thanks for the feedback!
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39389554
BTW I've updated it now to be just your initial commit, but without attempting to remove IPYTHON_OPTS. I think this is the best solution.
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39388854
I see, in that case, I think we can do the following:
* Leave in IPYTHON_OPTS as a way to pass options to IPython. Otherwise the IPython Notebook won't work, and neither will the Pylab flags and stuff like that.
* Add back the number of arguments = 0 check you added.
* In later versions of IPython that fix its startup bug, we can remove that check and let you run a script through IPython too. I guess you can also do `IPYTHON_OPTS="myscript.py" bin/pyspark`.
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39396271
@bouk that's great, thanks, but we probably can't have it be the default for a while until more people update their IPython.
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39395521
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13700/
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39287959
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13660/
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/294
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39284727
Merged build triggered.
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39287958
Merged build finished. All automated tests 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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39389698
Merged build triggered.
---
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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by bouk <gi...@git.apache.org>.
Github user bouk commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39391545
Note that the new version of IPython released today has my fix in it, so doing `exec ipython $@` should be fine with version 2.0.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.
---
[GitHub] spark pull request: [SPARK-1134] Fix and document passing of argum...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/294#issuecomment-39284732
Merged build started.
---
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.
---