You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by aarondav <gi...@git.apache.org> on 2014/03/28 23:13:25 UTC

[GitHub] spark pull request: SPARK-1349: spark-shell gets its own command h...

GitHub user aarondav opened a pull request:

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

    SPARK-1349: spark-shell gets its own command history

    Currently, spark-shell shares its command history with scala repl.
    
    This fix is simply a modification of the default FileBackedHistory file setting:
    https://github.com/scala/scala/blob/master/src/repl/scala/tools/nsc/interpreter/session/FileBackedHistory.scala#L77

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

    $ git pull https://github.com/aarondav/spark repl

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

    https://github.com/apache/spark/pull/267.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 #267
    
----
commit f9c62d21fea922207e182e48e946715172366a84
Author: Aaron Davidson <aa...@databricks.com>
Date:   2014-03-28T22:03:24Z

    SPARK-1349: spark-shell gets its own command history separate from scala repl
    
    This fix is simply a modification of the default FileBackedHistory file setting:
    https://github.com/scala/scala/blob/master/src/repl/scala/tools/nsc/interpreter/session/FileBackedHistory.scala#L77

----


---
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-1349: spark-shell gets its own command h...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/267#discussion_r11323765
  
    --- Diff: repl/src/main/scala/org/apache/spark/repl/SparkJLineReader.scala ---
    @@ -78,3 +80,11 @@ class SparkJLineReader(_completion: => Completion) extends InteractiveReader {
       def readOneLine(prompt: String) = consoleReader readLine prompt
       def readOneKey(prompt: String)  = consoleReader readOneKey prompt
     }
    +
    +/** Changes the default history file to not collide with the scala repl's. */
    +class SparkJLineHistory extends JLineFileHistory {
    +  import Properties.userHome
    +
    +  def defaultFileName = ".spark_history"
    +  override protected lazy val historyFile = File(Path(userHome) / defaultFileName)
    --- End diff --
    
    Okay sounds good - just wondering why it was there.


---
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-1349: spark-shell gets its own command h...

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

    https://github.com/apache/spark/pull/267#issuecomment-38974160
  
    Merged build started. Build is starting -or- tests failed to complete.


---
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-1349: spark-shell gets its own command h...

Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/267#discussion_r11099127
  
    --- Diff: repl/src/main/scala/org/apache/spark/repl/SparkJLineReader.scala ---
    @@ -78,3 +80,11 @@ class SparkJLineReader(_completion: => Completion) extends InteractiveReader {
       def readOneLine(prompt: String) = consoleReader readLine prompt
       def readOneKey(prompt: String)  = consoleReader readOneKey prompt
     }
    +
    +/** Changes the default history file to not collide with the scala repl's. */
    +class SparkJLineHistory extends JLineFileHistory {
    +  import Properties.userHome
    +
    +  def defaultFileName = ".spark_history"
    +  override protected lazy val historyFile = File(Path(userHome) / defaultFileName)
    --- End diff --
    
    Just wondering - does this handle systems with different path separators correct?
    
    Does `historyFile` need to be  `scala.reflect.io.File`? Or can it be a `java.io.File`? It seems a little heavyweight to pull in the reflection library to create a file path, but it makes sense if this is the needed type of `historyFile`.


---
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-1349: spark-shell gets its own command h...

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

    https://github.com/apache/spark/pull/267#issuecomment-38974154
  
     Merged build triggered. Build is starting -or- tests failed to complete.


---
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-1349: spark-shell gets its own command h...

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

    https://github.com/apache/spark/pull/267#issuecomment-39688636
  
    Thanks, merged.


---
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-1349: spark-shell gets its own command h...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/267#discussion_r11323731
  
    --- Diff: repl/src/main/scala/org/apache/spark/repl/SparkJLineReader.scala ---
    @@ -78,3 +80,11 @@ class SparkJLineReader(_completion: => Completion) extends InteractiveReader {
       def readOneLine(prompt: String) = consoleReader readLine prompt
       def readOneKey(prompt: String)  = consoleReader readOneKey prompt
     }
    +
    +/** Changes the default history file to not collide with the scala repl's. */
    +class SparkJLineHistory extends JLineFileHistory {
    +  import Properties.userHome
    +
    +  def defaultFileName = ".spark_history"
    +  override protected lazy val historyFile = File(Path(userHome) / defaultFileName)
    --- End diff --
    
    The reason I did this was to fully mimic the [code from the Scala interpreter](https://github.com/scala/scala/blob/master/src/repl/scala/tools/nsc/interpreter/session/FileBackedHistory.scala#L77). I think this will make it easier to compare in case the Scala version is updated some time in the future, but you're right we could use a lighter weight construct, we just risk having different behavior than the standard Scala shell history.
    
    Not a big deal to me, but I'm inclined to stick with cloning the Scala shell behavior. Let me know if you still prefer the `java.io.File` route, I'd be happy to change to that.


---
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-1349: spark-shell gets its own command h...

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

    https://github.com/apache/spark/pull/267#issuecomment-38980299
  
    Jenkins, retest this please.


---
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-1349: spark-shell gets its own command h...

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

    https://github.com/apache/spark/pull/267#issuecomment-38977253
  
    Merged build finished. Build is starting -or- tests failed to complete.


---
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-1349: spark-shell gets its own command h...

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

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


---
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-1349: spark-shell gets its own command h...

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

    https://github.com/apache/spark/pull/267#issuecomment-38977254
  
    Build is starting -or- tests failed to complete.
    Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13563/


---
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-1349: spark-shell gets its own command h...

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

    https://github.com/apache/spark/pull/267#issuecomment-38982191
  
    All automated tests passed.
    Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13564/


---
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-1349: spark-shell gets its own command h...

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

    https://github.com/apache/spark/pull/267#issuecomment-38982190
  
    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-1349: spark-shell gets its own command h...

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

    https://github.com/apache/spark/pull/267#issuecomment-38980392
  
     Merged build triggered. Build is starting -or- tests failed to complete.


---
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-1349: spark-shell gets its own command h...

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

    https://github.com/apache/spark/pull/267#issuecomment-38980398
  
    Merged build started. Build is starting -or- tests failed to complete.


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