You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spark.apache.org by CodingCat <gi...@git.apache.org> on 2014/02/18 17:29:40 UTC
[GitHub] incubator-spark pull request: [SPARK-1089] fix the problem in 0.9 ...
GitHub user CodingCat opened a pull request:
https://github.com/apache/incubator-spark/pull/614
[SPARK-1089] fix the problem in 0.9 that ADD_JARS value was not recognized
https://spark-project.atlassian.net/browse/SPARK-1089
load jar in process() and work around for scala issue
The reason of this bug is two-folds
1. in the current implementation of SparkILoop.scala, the settings.classpath is not set properly when the process() method is invoked
2. the weird behaviour of Scala 2.10, (I personally thought it is a bug)
if we simply set value of a PathSettings object (like settings.classpath), the isDefault is not set to true (this is a flag showing if the variable is modified), so it makes the PathResolver loads the default CLASSPATH environment variable value to calculated the path (see https://github.com/scala/scala/blob/2.10.x/src/compiler/scala/tools/util/PathResolver.scala#L215)
what we have to do is to manually make this flag set, (https://github.com/CodingCat/incubator-spark/blob/e3991d97ddc33e77645e4559b13bf78b9e68239a/repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala#L884)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/CodingCat/incubator-spark SPARK-1089
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-spark/pull/614.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 #614
----
commit e3991d97ddc33e77645e4559b13bf78b9e68239a
Author: CodingCat <zh...@gmail.com>
Date: 2014-02-18T16:23:15Z
load jar in process() and work around for scala issue
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35981333
there is no need to pass it there, you can do
```scala
val totalClasspath = ClassPath.join(settings.classpath.value, addedClasspath)
settings.classpath.value = totalClasspath
```
where added classpath is one we get by joining SparkILoop.getAddedJars as above.
Haven't tried it myself. Just see if it works.
---
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/614#discussion_r9972360
--- Diff: repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
@@ -876,7 +876,14 @@ class SparkILoop(in0: Option[BufferedReader], protected val out: JPrintWriter,
})
def process(settings: Settings): Boolean = savingContextLoader {
+
+ SparkILoop.getAddedJars.foreach(settings.classpath.append(_))
this.settings = settings
+ //work around for Scala bug
+ if (settings.classpath != null)
+ this.settings.classpath.value_= (settings.classpath.value)
--- End diff --
Hi, @pwendell , actually it is value_= ...(weird name..., but it is the only way to make setByUser to be true....)
def value_=(arg: T) = {
setByUser = true
v = arg
postSetHook()
}
so
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35880521
@ScrapCodes thank you very much
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/614#discussion_r9972843
--- Diff: repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
@@ -876,7 +876,14 @@ class SparkILoop(in0: Option[BufferedReader], protected val out: JPrintWriter,
})
def process(settings: Settings): Boolean = savingContextLoader {
+
+ SparkILoop.getAddedJars.foreach(settings.classpath.append(_))
this.settings = settings
+ // work around for Scala bug
+ if (settings.classpath != null)
+ this.settings.classpath.value_= (settings.classpath.value)
--- End diff --
This is actually a proper setter (see http://dustinmartin.net/getters-and-setters-in-scala/) and you should be able to do
`this.settings.classpath.value = (settings.classpath.value)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35809685
// @scrapcodes - mind reviewing this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/614#discussion_r9972883
--- Diff: repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
@@ -876,7 +876,14 @@ class SparkILoop(in0: Option[BufferedReader], protected val out: JPrintWriter,
})
def process(settings: Settings): Boolean = savingContextLoader {
+
+ SparkILoop.getAddedJars.foreach(settings.classpath.append(_))
this.settings = settings
+ // work around for Scala bug
+ if (settings.classpath != null)
+ this.settings.classpath.value_= (settings.classpath.value)
--- End diff --
I tried it before, I remember this much more straightforward way doesn't work...I will try it again right now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the problem in 0.9 ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35402826
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. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35859704
Nice catch ! and thanks for taking the time to dig this. I am okay with this way of doing it, however if you and others prefer we can move this code to createInterpreter before creating SparkILoopInterpreter. Even if we don't I think its fine to merge it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/614#discussion_r9972921
--- Diff: repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
@@ -876,7 +876,14 @@ class SparkILoop(in0: Option[BufferedReader], protected val out: JPrintWriter,
})
def process(settings: Settings): Boolean = savingContextLoader {
+
+ SparkILoop.getAddedJars.foreach(settings.classpath.append(_))
this.settings = settings
+ // work around for Scala bug
+ if (settings.classpath != null)
+ this.settings.classpath.value_= (settings.classpath.value)
--- End diff --
it works....what happened several days ago.........
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35806096
any discussion on this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/614#discussion_r9972286
--- Diff: repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
@@ -876,7 +876,14 @@ class SparkILoop(in0: Option[BufferedReader], protected val out: JPrintWriter,
})
def process(settings: Settings): Boolean = savingContextLoader {
+
+ SparkILoop.getAddedJars.foreach(settings.classpath.append(_))
this.settings = settings
+ //work around for Scala bug
--- End diff --
Needs a space
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by CodingCat <gi...@git.apache.org>.
Github user CodingCat commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35908324
@ScrapCodes I'm looking at your suggestion, the difficulty to move to createInterpreter() is that you cannot not pass the parameter "settings" to there, do you want me to change the createInterpreter() API (I don't think so)?
---
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35811286
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/12812/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35809765
Jenkins, test 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. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35809789
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. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35811284
Merged build finished.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35809788
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. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:
https://github.com/apache/incubator-spark/pull/614#issuecomment-35862636
Also from this I just went ahead and tried fixing this problem in scala and it worked.
https://github.com/ScrapCodes/scala/tree/si-6502-fix
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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] incubator-spark pull request: [SPARK-1089] fix the regression prob...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/incubator-spark/pull/614#discussion_r9972298
--- Diff: repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala ---
@@ -876,7 +876,14 @@ class SparkILoop(in0: Option[BufferedReader], protected val out: JPrintWriter,
})
def process(settings: Settings): Boolean = savingContextLoader {
+
+ SparkILoop.getAddedJars.foreach(settings.classpath.append(_))
this.settings = settings
+ //work around for Scala bug
+ if (settings.classpath != null)
+ this.settings.classpath.value_= (settings.classpath.value)
--- End diff --
Space
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. To do so, please top-post your response.
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.
---