You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by Tarrasch <gi...@git.apache.org> on 2015/11/12 04:11:24 UTC

[GitHub] spark pull request: [MINOR][DOCS] quick-start: Escape shell comman...

GitHub user Tarrasch opened a pull request:

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

    [MINOR][DOCS] quick-start: Escape shell commands

    When using the (very popular) zsh shell, running commands like `echo local[4]` will result in an error as the brackets will be interpreted by the shell. This patch escapes those in single quotes for the quick start tutorial.
    
    This patch also removes the unnecessary quoting of the `--class` parameter as it is not needed and cluttery.

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

    $ git pull https://github.com/Tarrasch/spark patch-1

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

    https://github.com/apache/spark/pull/9647.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 #9647
    
----
commit f73f49caf09766e564128ef8be0f0f59be1c8009
Author: Arash Rouhani <mi...@gmail.com>
Date:   2015-11-12T03:11:04Z

    [MINOR][DOCS] quick-start: Escape shell commands
    
    When using the (very popular) zsh shell, running commands like `echo local[4]` will result in an error as the brackets will be interpreted by the shell. This patch escapes those in single quotes for the quick start tutorial.
    
    This patch also removes the unnecessary quoting of the `--class` parameter as it is not needed and cluttery.

----


---
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: [MINOR][DOCS] quick-start: Escape shell comman...

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

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


---
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: [MINOR][DOCS] quick-start: Escape shell comman...

Posted by Tarrasch <gi...@git.apache.org>.
Github user Tarrasch commented on the pull request:

    https://github.com/apache/spark/pull/9647#issuecomment-156706664
  
    I'm not going to go through all docs and update them. That this simple patch have spawned so much discussion is not worth my time. Clearly this increases compatibilities for all shells in all modes. It's solely positive.
    
    But if your philosophy is either fix all bugs or fix no bugs. Then I'm giving up.


---
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: [MINOR][DOCS] quick-start: Escape shell comman...

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

    https://github.com/apache/spark/pull/9647#issuecomment-156084719
  
    I don't know that in general Spark scripts work at all with `zsh`. Why single quote and not double? I think there are many more instances you'd have to change too, not just this one.
    
    In the end, won't the few `zsh` users out there immediately understand the variance and fix it? It might not be worth the friction for the supported `bash` users out there, when I don't think we achieve `zsh` compatibility in general.


---
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: [MINOR][DOCS] quick-start: Escape shell comman...

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

    https://github.com/apache/spark/pull/9647#issuecomment-156709368
  
    I can't close your PR, so please close it yourself. 


---
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: [MINOR][DOCS] quick-start: Escape shell comman...

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

    https://github.com/apache/spark/pull/9647#issuecomment-156362518
  
    Here, we are talking about modifying shell commands presented in docs, not in `.sh` files. Your point is that the shell _does_ matter right? 
    
    The `.sh` files in Spark all require `bash` -- which is neither here nor there in that you can invoke a `bash` script from `zsh` just fine, yes. But all of the rest of the Spark docs likewise probably have incompatibilities with `zsh`. It's not-unrelated to the fact that Spark internals only work with `bash`.
    
    This would at least need to check and fix any other incompatibilities in the docs. At that point I wonder if it's worth the little extra friction for most users, who will be using `bash`. I'm concerned it's indirectly setting an expectation that all of Spark could be fine without `bash` installed.


---
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: [MINOR][DOCS] quick-start: Escape shell comman...

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

    https://github.com/apache/spark/pull/9647#issuecomment-155984484
  
    Can one of the admins verify this patch?


---
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: [MINOR][DOCS] quick-start: Escape shell comman...

Posted by Tarrasch <gi...@git.apache.org>.
Github user Tarrasch commented on the pull request:

    https://github.com/apache/spark/pull/9647#issuecomment-156347814
  
    > I don't know that in general Spark scripts work at all with zsh.
    
    You launch a binary, you don't source a script. The shell you invoke a binary from doesn't matter. So yes, given that the scripts are passed the correct parameters, it works from any shell.
    
    > Why single quote and not double?
    
    With single quotes it is clear that there is no variable interpolation going on. This is not my own convention. Puppet's linter for example rejects double-quotes if you don't interpolate variables.
    
    > In the end, won't the few zsh users out there immediately understand the variance and fix it?
    
    Yea. In this Pull Request I'm fixing it.
    
    Besides. When you say "it works for bash". You mean "it works for bash default options". The same with zsh. I'm sure bash will not accept the unquoted `local[4]` in some modes. Adding the single quotes should make it work for all modes in all (sensible) shells.


---
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: [MINOR][DOCS] quick-start: Escape shell comman...

Posted by Tarrasch <gi...@git.apache.org>.
Github user Tarrasch commented on the pull request:

    https://github.com/apache/spark/pull/9647#issuecomment-156769218
  
    @srowen sent me an email pointing out I could have a better manner. He is of course correct and I thank him for his time. :)
    
    As for this PR. Let's close it as it seems to be jinxed with misunderstanding. Hopefully my next patch will go more smooth. :)


---
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