You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by pierre-borckmans <gi...@git.apache.org> on 2014/04/30 18:06:52 UTC
[GitHub] spark pull request: Remove hardcoded Spark version string to use S...
GitHub user pierre-borckmans opened a pull request:
https://github.com/apache/spark/pull/600
Remove hardcoded Spark version string to use SBT defined version instead
This PR attempts to remove hardcoded references to the Spark version which must be updated for each release.
This PR uses the version defined in the main sbt definition file (project/SparkBuild.scala - ``SPARK_VERSION``). When the sbt compile/package/assembly is triggered, this string is used as the version in JAR manifest.
We then use ``getClass.getPackage.getImplementationVersion`` to refer to this string.
As a demonstration of potential use cases, the master ui page is modified to display the Spark version.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pierre-borckmans/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/600.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 #600
----
commit 12769f4e7a8ab564c01c0a9a98f48f8fbbf7642c
Author: pierre-borckmans <pi...@realimpactanalytics.com>
Date: 2014-04-30T15:52:12Z
Remove hardcoded Spark version - use the version from the manifest (SBT) instead
commit f7c2298a7769d83fe52b6e9516d2b3ebfc219056
Author: pierre-borckmans <pi...@realimpactanalytics.com>
Date: 2014-04-30T15:52:58Z
Add spark version label in ui (master index page)
commit d26bc1e8d4cf7f2e39416824728765ca14053980
Author: pierre-borckmans <pi...@realimpactanalytics.com>
Date: 2014-04-30T15:53:51Z
Merge remote-tracking branch 'upstream/master'
----
---
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: Remove hardcoded Spark version string to use S...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-50693514
I think we should close this issue, it's gone stale
---
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: Remove hardcoded Spark version string to use S...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/600#discussion_r12152899
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -1037,6 +1037,11 @@ private[spark] object Utils extends Logging {
obj.getClass.getSimpleName.replace("$", "")
}
+ /** Return the current version of Spark */
+ def getSparkVersion(): String = {
+ getClass.getPackage.getImplementationVersion
--- End diff --
Does this also work in the maven build?
---
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: Remove hardcoded Spark version string to use S...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/600
---
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: Remove hardcoded Spark version string to use S...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-42116524
@pierre-borckmans Appreciate you looking into this, but I really don't think it's worth doing anything complex here - we just just leave it hard coded.
We already have to change a few things each time we release Spark... this really isn't a big concern for us. And bringing in build plug-ins etc is just an over-engineered solution here.
---
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: Remove hardcoded Spark version string to use S...
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41819150
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. 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: Remove hardcoded Spark version string to use S...
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:
https://github.com/apache/spark/pull/600#discussion_r12159750
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -1037,6 +1037,11 @@ private[spark] object Utils extends Logging {
obj.getClass.getSimpleName.replace("$", "")
}
+ /** Return the current version of Spark */
+ def getSparkVersion(): String = {
+ getClass.getPackage.getImplementationVersion
--- End diff --
Note that this manifest property must be correctly set from the build system, but it's good to know if both systems do 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] spark pull request: Remove hardcoded Spark version string to use S...
Posted by pierre-borckmans <gi...@git.apache.org>.
Github user pierre-borckmans commented on a diff in the pull request:
https://github.com/apache/spark/pull/600#discussion_r12151374
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -1253,7 +1253,7 @@ class SparkContext(config: SparkConf) extends Logging {
*/
object SparkContext extends Logging {
- private[spark] val SPARK_VERSION = "1.0.0"
+ private[spark] val SPARK_VERSION = getClass.getPackage.getImplementationVersion
--- End diff --
This should be be:
```
private[spark] val SPARK_VERSION = Utils.getSparkVersion
```
instead, I forgot to update it.
---
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: Remove hardcoded Spark version string to use S...
Posted by pierre-borckmans <gi...@git.apache.org>.
Github user pierre-borckmans commented on a diff in the pull request:
https://github.com/apache/spark/pull/600#discussion_r12156040
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -1037,6 +1037,11 @@ private[spark] object Utils extends Logging {
obj.getClass.getSimpleName.replace("$", "")
}
+ /** Return the current version of Spark */
+ def getSparkVersion(): String = {
+ getClass.getPackage.getImplementationVersion
--- End diff --
I did not check as I was not really aware of the Maven build.
Is there any doc about this?
---
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: Remove hardcoded Spark version string to use S...
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41835242
Unrelated test error, jenkins retest this.
---
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: Remove hardcoded Spark version string to use S...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41823752
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14588/
---
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: Remove hardcoded Spark version string to use S...
Posted by pierre-borckmans <gi...@git.apache.org>.
Github user pierre-borckmans commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41852359
A last idea.
How about this sbt plugin?
https://github.com/sbt/sbt-buildinfo
It could definitely do the trick.
We would still need sth equivalent for maven.
Message sent from a mobile service - excuse typos and abbreviations
> Le 30 avr. 2014 à 21:58, Patrick Wendell <no...@github.com> a écrit :
>
> @pierre-borckmans ah okay makes sense. I think in this case hard coding might be the best of (admittedly not great) alternatives. No problem on overlooking those cases, they are pretty non-obvious!
>
> —
> Reply to this email directly or view it on GitHub.
---
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: Remove hardcoded Spark version string to use S...
Posted by pierre-borckmans <gi...@git.apache.org>.
Github user pierre-borckmans commented on a diff in the pull request:
https://github.com/apache/spark/pull/600#discussion_r12151396
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -1565,4 +1565,3 @@ private[spark] class WritableConverter[T](
val writableClass: ClassTag[T] => Class[_ <: Writable],
val convert: Writable => T)
extends Serializable
-
--- End diff --
Unintended removal of blank line
---
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: Remove hardcoded Spark version string to use S...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41840032
(sorry for the comment bomb). Okay I looked a bit more... it seems like this relies on the jar manifest. While it's definitely nice to not hard-code the version, I see some potential concerns:
1. Someone is running spark without a jar file (e.g. our own unit tests, when we run spark locally with assemble-deps, etc).
2. Someone has embedded spark in their own assembly jar, in which case the manifest is not going to inherit from Spark.
In case 2 I think this might silently give the wrong version, i.e. it could just learn the version from whoever built the assembly.
So it might be worth it to just keep this hard-coded for now... we have dependencies that rely on META-INF configurations (e.g. datanucleus) and it's pretty annoying.
---
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: Remove hardcoded Spark version string to use S...
Posted by pierre-borckmans <gi...@git.apache.org>.
Github user pierre-borckmans commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41840133
@pwendell AFAIK it's only made available in the META-INF of the fat jar. So you are right.
Sorry about that, I overlooked these 2 scenarios.
---
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: Remove hardcoded Spark version string to use S...
Posted by pierre-borckmans <gi...@git.apache.org>.
Github user pierre-borckmans commented on a diff in the pull request:
https://github.com/apache/spark/pull/600#discussion_r12156870
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -1037,6 +1037,11 @@ private[spark] object Utils extends Logging {
obj.getClass.getSimpleName.replace("$", "")
}
+ /** Return the current version of Spark */
+ def getSparkVersion(): String = {
+ getClass.getPackage.getImplementationVersion
--- End diff --
It seems to work as well.
In fact, the build system does not matter:
http://docs.oracle.com/javase/7/docs/api/java/lang/Package.html
The version string is retrieved from the JAR manifest.
---
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: Remove hardcoded Spark version string to use S...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41838753
@pierre-borckmans @aarondav what happens if someone is just running compiled spark classes (i.e. they aren't running Spark in an environment where it's been bundled as a jar). Also, what about if someone else has embedded Spark code inside of their assembly jar, similar to how we do it with our dependencies?
Will this still work?
---
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: Remove hardcoded Spark version string to use S...
Posted by mridulm <gi...@git.apache.org>.
Github user mridulm commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-42115749
Please ensure it works in both sbt and maven case.
---
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: Remove hardcoded Spark version string to use S...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41819533
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: Remove hardcoded Spark version string to use S...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41815619
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.
---
[GitHub] spark pull request: Remove hardcoded Spark version string to use S...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41823751
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. 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: Remove hardcoded Spark version string to use S...
Posted by pierre-borckmans <gi...@git.apache.org>.
Github user pierre-borckmans commented on a diff in the pull request:
https://github.com/apache/spark/pull/600#discussion_r12156082
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -1037,6 +1037,11 @@ private[spark] object Utils extends Logging {
obj.getClass.getSimpleName.replace("$", "")
}
+ /** Return the current version of Spark */
+ def getSparkVersion(): String = {
--- End diff --
thanks, i'll fix this
---
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: Remove hardcoded Spark version string to use S...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41819524
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: Remove hardcoded Spark version string to use S...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41842829
@pierre-borckmans ah okay makes sense. I think in this case hard coding might be the best of (admittedly not great) alternatives. No problem on overlooking those cases, they are pretty non-obvious!
---
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: Remove hardcoded Spark version string to use S...
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:
https://github.com/apache/spark/pull/600#discussion_r12152601
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -1037,6 +1037,11 @@ private[spark] object Utils extends Logging {
obj.getClass.getSimpleName.replace("$", "")
}
+ /** Return the current version of Spark */
+ def getSparkVersion(): String = {
--- End diff --
If you intend it to be called without parentheses, it should also be defined without them.
---
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: Remove hardcoded Spark version string to use S...
Posted by pierre-borckmans <gi...@git.apache.org>.
Github user pierre-borckmans commented on a diff in the pull request:
https://github.com/apache/spark/pull/600#discussion_r12160496
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -1037,6 +1037,11 @@ private[spark] object Utils extends Logging {
obj.getClass.getSimpleName.replace("$", "")
}
+ /** Return the current version of Spark */
+ def getSparkVersion(): String = {
+ getClass.getPackage.getImplementationVersion
--- End diff --
Indeed.
I confirm that both ``sbt assembly`` and ``mvn package``produce a fat jar in which the manifest contains the version.
```
unzip -p assembly/target/scala-2.10/spark-assembly-1.0.0-SNAPSHOT-hadoop2.0.0-mr1-cdh4.4.0.jar META-INF/MANIFEST.MF
```
outputs:
```
Implementation-Vendor: The Apache Software Foundation
Implementation-Title: Spark Project Core
Implementation-Version: 1.0.0-SNAPSHOT
Implementation-Vendor-Id: org.apache.spark
Built-By: ria
Build-Jdk: 1.6.0_27
Specification-Vendor: The Apache Software Foundation
Specification-Title: Spark Project Core
Created-By: Apache Maven 3.0.4
Specification-Version: 1.0.0-SNAPSHOT
Archiver-Version: Plexus Archiver
```
in both cases
---
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: Remove hardcoded Spark version string to use S...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/600#issuecomment-41839022
Basically my question is whether this is embedded somewhere in the jar packaging (e.g. META-INF) or if it's actually in the byte code of each spark class. If it's the former, I don't see how it will work in the two cases I mentioned before.
---
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.
---