[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[jodersky]

Github user jodersky commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-222380888
  
What I understand from the conversation is that maven assumes the 
target/extra-resources directory:
> Unfortunately I'm not an maven expert but I wonder if we can put it in 
like target/extra-resources and have it automatically picked up still. 
http://maven.apache.org/plugins/maven-resources-plugin/examples/copy-resources.html

I'm suggesting to use `resourceManaged` in sbt, since I have had past 
problems with resources not getting picked up automatically when they were not 
in that directory.

It's a minor detail and if everything works for you, than this LGTM too.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-222335144
  
@jodersky  you can get the context if you follow the conversation as to why 
its placed in target/extra-resources.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[jodersky]

Github user jodersky commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r64942108
  
--- Diff: project/SparkBuild.scala ---
@@ -436,7 +439,19 @@ object SparkBuild extends PomBuild {
   else x.settings(Seq[Setting[_]](): _*)
 } ++ Seq[Project](OldDeps.project)
   }
+}
 
+object Core {
+  lazy val settings = Seq(
+resourceGenerators in Compile += Def.task {
+  val buildScript = baseDirectory.value + "/../build/spark-build-info"
+  val targetDir = baseDirectory.value + "/target/extra-resources/"
+  val command =  buildScript + " " + targetDir + " " + version.value
+  Process(command).!!
+  val propsFile = baseDirectory.value / "target" / "extra-resources" / 
"spark-version-info.properties"
--- End diff --

Maybe a nit: shouldn't all resources be generated into `resourceManaged`? 
I'm not sure how sbt will handle packaging of non-resourcemanaged artifacts; it 
could be that spark works locally, however errors may occur in a packaged 
environment.

I would suggest renaming `"target"/"extra-resources"` to `(resourceManaged 
in Compile).value`. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-222187952
  
@vanzin , @tgravescs If the changes are good to go, can we merge them in. 

We could run the jenkins test once again, with all the changes in. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r64667967
  
--- Diff: build/spark-build-info ---
@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# This script generates the build info for spark and places it into the 
spark-version-info.properties file.
+# Arguments:
+#   build_tgt_directory - The target directory where properties file would 
be created. [./core/target/extra-resources]
+#   spark_version - The current version of spark
+
+RESOURCE_DIR=$1
--- End diff --

Sorry, should have caught this earlier, but you need quotes around 
variables that might contain spaces.

e.g. `RESOURCE_DIR="$1"`

Same thing later on, e.g. `mkdir -p "$RESOURCE_DIR"`.



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-221724401
  
@vanzin - I have done the suggested refactoring. Thanks for reviewing.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r64657683
  
--- Diff: core/src/main/scala/org/apache/spark/package.scala ---
@@ -41,7 +41,46 @@ package org.apache
  * level interfaces. These are subject to changes or removal in minor 
releases.
  */
 
+import java.util.Properties
+
 package object spark {
-  // For package docs only
-  val SPARK_VERSION = "2.0.0-SNAPSHOT"
+
+  private object SparkBuildInfo {
+val unknownProp = ""
+val props = new Properties()
+val resourceStream = Thread.currentThread().getContextClassLoader.
+  getResourceAsStream("spark-version-info.properties")
+val (spark_version: String, spark_branch: String, spark_revision: 
String,
+  spark_build_user: String, spark_repo_url: String, spark_build_date: 
String) =
+  try {
+props.load(resourceStream)
+(
+  props.getProperty("version", unknownProp),
+  props.getProperty("branch", unknownProp),
+  props.getProperty("revision", unknownProp),
+  props.getProperty("user", unknownProp),
+  props.getProperty("url", unknownProp),
+  props.getProperty("date", unknownProp)
+)
+  } catch {
+case e: Exception =>
+  throw new SparkException("Error loading properties from 
spark-version-info.properties", e)
+  } finally {
+if (resourceStream != null) {
+  try {
+resourceStream.close()
+  } catch {
+case e: Exception =>
+  throw new SparkException("Error closing spark build info 
resource stream", e)
+  }
+}
+  }
+  }
+  val SPARK_VERSION = SparkBuildInfo.spark_version
--- End diff --

nit: add empty line 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-221716206
  
Just style nits, otherwise looks good.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r64658119
  
--- Diff: core/src/main/scala/org/apache/spark/package.scala ---
@@ -41,7 +41,46 @@ package org.apache
  * level interfaces. These are subject to changes or removal in minor 
releases.
  */
 
+import java.util.Properties
+
 package object spark {
-  // For package docs only
-  val SPARK_VERSION = "2.0.0-SNAPSHOT"
+
+  private object SparkBuildInfo {
+val unknownProp = ""
+val props = new Properties()
+val resourceStream = Thread.currentThread().getContextClassLoader.
+  getResourceAsStream("spark-version-info.properties")
+val (spark_version: String, spark_branch: String, spark_revision: 
String,
+  spark_build_user: String, spark_repo_url: String, spark_build_date: 
String) =
--- End diff --

Could you place the temp `val`s inside a scoped block here, so they can be 
garbage collected? e.g.:

```
val (spark_version: String, spark_branch: String, spark_revision: String,
  spark_build_user: String, spark_repo_url: String, spark_build_date: 
String) = {

  val unknownProp = ""
  ...
  try {
...
  } 
}
```

Also, when you have multiple "parameters", we generally do the following:

```
val (
spark_version: String, 
spark_branch: String, 
spark_revision: String,
spark_build_user: String,
spark_repo_url: String,
spark_build_date: String) = {
  // code
}
```



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r64657618
  
--- Diff: build/spark-build-info ---
@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# This script generates the build info for spark and places it into the 
spark-version-info.properties file.
+# Arguments:
+#   build_tgt_directory - The target directory where properties file would 
be created. [./core/target/extra-resources]
+#   spark_version - The current version of spark
+
+RESOURCE_DIR=$1
+mkdir -p $RESOURCE_DIR
+SPARK_BUILD_INFO=${RESOURCE_DIR}/spark-version-info.properties
+
+echo_build_properties(){
--- End diff --

nit: space after `()`. Also we use 2-space indent in shell scripts.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-221713163
  
@vanzin can you review the changes. - Tx


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[jodersky]

Github user jodersky commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r6396
  
--- Diff: project/SparkBuild.scala ---
@@ -435,7 +438,19 @@ object SparkBuild extends PomBuild {
   else x.settings(Seq[Setting[_]](): _*)
 } ++ Seq[Project](OldDeps.project)
   }
+}
 
+object Core {
+  lazy val settings = Seq(
+unmanagedResourceDirectories in Compile += baseDirectory.value / 
"target" / "extra-resources",
+resourceGenerators in Compile += Def.task {
+  val buildScript = baseDirectory.value + "/../build/spark-build-info"
+  val targetDir = baseDirectory.value + "/target/extra-resources/"
+  val command =  buildScript + " " + targetDir + " " + version.value
+  Process(Seq("/bin/bash", "-c", command)).!!
--- End diff --

Is the extra unmanagedResourceDirectory required? Alternatively, the 
resource generator could output the extra properties file into the 
`resourceManaged` directory (defaults to "resource_managed"), that way it would 
also automatically be picked up in the classpath.

Btw, I think that generating the file into `resourceManaged` would solve 
the problem with clean you reported on the mailing list.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63431109
  
--- Diff: core/pom.xml ---
@@ -335,9 +335,38 @@
   
 
target/scala-${scala.binary.version}/classes
 
target/scala-${scala.binary.version}/test-classes
+
+  
+
+${project.build.directory}/extra-resources
+true
+  
+
 
   
 org.apache.maven.plugins
+maven-antrun-plugin
+${maven-antrun.version}
+
+  
+generate-resources
+
+  
+  
+
--- End diff --

Now that we have switched to executing a shell script, we don't require 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63414568
  
--- Diff: core/pom.xml ---
@@ -335,9 +335,38 @@
   
 
target/scala-${scala.binary.version}/classes
 
target/scala-${scala.binary.version}/test-classes
+
+  
+
+${project.build.directory}/extra-resources
+true
+  
+
 
   
 org.apache.maven.plugins
+maven-antrun-plugin
+${maven-antrun.version}
--- End diff --

Yes. We can get rid of the version here. However I don't see any harm 
adding a single entry like other version properties for a plugin. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63411773
  
--- Diff: core/src/main/scala/org/apache/spark/package.scala ---
@@ -41,7 +41,50 @@ package org.apache
  * level interfaces. These are subject to changes or removal in minor 
releases.
  */
 
+import java.util.Properties
+
+import org.apache.spark.internal.Logging
+
 package object spark {
-  // For package docs only
-  val SPARK_VERSION = "2.0.0-SNAPSHOT"
+
+  object SparkBuildInfo extends Logging {
+val unknownProp = ""
+val props = new Properties()
+val resourceStream = Thread.currentThread().getContextClassLoader.
+  getResourceAsStream("spark-version-info.properties")
+val (spark_version: String, spark_branch: String, spark_revision: 
String,
+  spark_build_user: String, spark_repo_url: String, spark_build_date: 
String) =
+  try {
+props.load(resourceStream)
+(
+  props.getProperty("version", unknownProp),
+  props.getProperty("branch", unknownProp),
+  props.getProperty("revision", unknownProp),
+  props.getProperty("user", unknownProp),
+  props.getProperty("url", unknownProp),
+  props.getProperty("date", unknownProp)
+)
+  } catch {
+case e: Exception =>
+  logError("Unable to read spark build properties.", e);
+  throw new SparkException("Error load properties from 
spark-version-info.properties", e)
--- End diff --

Will correct this one. 

Agreed - Log seems redundant, we can remove 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63410648
  
--- Diff: core/src/main/scala/org/apache/spark/package.scala ---
@@ -41,7 +41,50 @@ package org.apache
  * level interfaces. These are subject to changes or removal in minor 
releases.
  */
 
+import java.util.Properties
+
+import org.apache.spark.internal.Logging
+
 package object spark {
-  // For package docs only
-  val SPARK_VERSION = "2.0.0-SNAPSHOT"
+
+  object SparkBuildInfo extends Logging {
--- End diff --

- Yes, the object is for a reason, its because of how internally Scala uses 
pattern matching. 

- You are right, that Scala doesn't have static initialization blocks like 
Java. And a nice/preferred way to construct and initialize the constants would 
be like what you showed which I did first while loading the values, however 
Scala compiler treats all cap variable names in this case as a class and not a 
variable and this is not obvious from the error message it throws. 

- You could test it in Scala REPL
```scala
// this one works fine
val (one, two) = ("one", "two")

// this one does not
val (THREE, FOUR) = ("three", "four")
```

- Making the object private makes sense. I will change 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63409645
  
--- Diff: build/spark-build-info ---
@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# This script generates the build info for spark and places it into the 
spark-version-info.properties file.
+# Arguments:
+#   build_tgt_directory - The target directory where properties file would 
be created. [./core/target/extra-resources]
+#   spark_version - The current version of spark
+
+RESOURCE_DIR=$1
+mkdir -p $RESOURCE_DIR
+SPARK_BUILD_INFO=${RESOURCE_DIR}/spark-version-info.properties
+
+(echo version=$2; echo user=$USER; echo revision=$(git rev-parse HEAD); 
echo branch=$(git rev-parse --abbrev-ref HEAD);
--- End diff --

Ok.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63401768
  
--- Diff: core/pom.xml ---
@@ -335,9 +335,38 @@
   
 
target/scala-${scala.binary.version}/classes
 
target/scala-${scala.binary.version}/test-classes
+
+  
+
+${project.build.directory}/extra-resources
+true
+  
+
 
   
 org.apache.maven.plugins
+maven-antrun-plugin
+${maven-antrun.version}
--- End diff --

Pretty sure you don't need to repeat the version here; which also makes 
your changes to the root pom unnecessary.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63401625
  
--- Diff: project/SparkBuild.scala ---
@@ -435,7 +438,19 @@ object SparkBuild extends PomBuild {
   else x.settings(Seq[Setting[_]](): _*)
 } ++ Seq[Project](OldDeps.project)
   }
+}
 
+object Core {
+  lazy val settings = Seq(
+unmanagedResourceDirectories in Compile += baseDirectory.value / 
"target" / "extra-resources",
+resourceGenerators in Compile += Def.task {
+  val buildScript = baseDirectory.value + "/../build/spark-build-info"
+  val targetDir = baseDirectory.value + "/target/extra-resources/"
+  val command =  buildScript + " " + targetDir + " " + version.value
+  Process(Seq("/bin/bash", "-c", command)).!!
--- End diff --

Same comment re: `bash -c`. Also, if users have bash somewhere else, this 
won't work (while the `/usr/bin/env bash` in the script would).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63401453
  
--- Diff: core/src/main/scala/org/apache/spark/package.scala ---
@@ -41,7 +41,50 @@ package org.apache
  * level interfaces. These are subject to changes or removal in minor 
releases.
  */
 
+import java.util.Properties
+
+import org.apache.spark.internal.Logging
+
 package object spark {
-  // For package docs only
-  val SPARK_VERSION = "2.0.0-SNAPSHOT"
+
+  object SparkBuildInfo extends Logging {
+val unknownProp = ""
+val props = new Properties()
+val resourceStream = Thread.currentThread().getContextClassLoader.
+  getResourceAsStream("spark-version-info.properties")
+val (spark_version: String, spark_branch: String, spark_revision: 
String,
+  spark_build_user: String, spark_repo_url: String, spark_build_date: 
String) =
+  try {
+props.load(resourceStream)
+(
+  props.getProperty("version", unknownProp),
+  props.getProperty("branch", unknownProp),
+  props.getProperty("revision", unknownProp),
+  props.getProperty("user", unknownProp),
+  props.getProperty("url", unknownProp),
+  props.getProperty("date", unknownProp)
+)
+  } catch {
+case e: Exception =>
+  logError("Unable to read spark build properties.", e);
+  throw new SparkException("Error load properties from 
spark-version-info.properties", e)
--- End diff --

nit: "loading"

The log also seems redundant, the exception should end up in some 
user-visible location already.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63401279
  
--- Diff: core/src/main/scala/org/apache/spark/package.scala ---
@@ -41,7 +41,50 @@ package org.apache
  * level interfaces. These are subject to changes or removal in minor 
releases.
  */
 
+import java.util.Properties
+
+import org.apache.spark.internal.Logging
+
 package object spark {
-  // For package docs only
-  val SPARK_VERSION = "2.0.0-SNAPSHOT"
+
+  object SparkBuildInfo extends Logging {
--- End diff --

Is this object necessary? It seems you already have explicit constants for 
everything, and instead this could just initialize them directly. Something 
like:

```
val (
SPARK_VERSION,
SPARK_BRANCH,
SPARK_REVISION,
SPARK_BUILD_USER,
SPARK_REPO_URL,
SPARK_BUILD_DATE) = {
  // Code to read the properties file.
}
```

Not super pretty, but about as pretty as your current code; iirc Scala 
doesn't really have static initializers like Java, which would make this a 
little nicer.

At the very least, this object should be private. I don't see a need to 
expose 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63400542
  
--- Diff: core/pom.xml ---
@@ -335,9 +335,38 @@
   
 
target/scala-${scala.binary.version}/classes
 
target/scala-${scala.binary.version}/test-classes
+
+  
+
+${project.build.directory}/extra-resources
+true
+  
+
 
   
 org.apache.maven.plugins
+maven-antrun-plugin
+${maven-antrun.version}
+
+  
+generate-resources
+
+  
+  
+
--- End diff --

Do you need to execute this through `bash -c`?

Make the script executable and it should just work. That should make things 
more readable since you can have each arg in its own line instead of a really 
long line with the command to execute.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63400095
  
--- Diff: build/spark-build-info ---
@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# This script generates the build info for spark and places it into the 
spark-version-info.properties file.
+# Arguments:
+#   build_tgt_directory - The target directory where properties file would 
be created. [./core/target/extra-resources]
+#   spark_version - The current version of spark
+
+RESOURCE_DIR=$1
+mkdir -p $RESOURCE_DIR
+SPARK_BUILD_INFO=${RESOURCE_DIR}/spark-version-info.properties
+
+(echo version=$2; echo user=$USER; echo revision=$(git rev-parse HEAD); 
echo branch=$(git rev-parse --abbrev-ref HEAD);
--- End diff --

Can you put this in a separate function instead?

```
echo_properties() {
  echo foo
  echo bar
  echo foobar
}

echo_properties > "$SPARK_BUILD_INFO"
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-219109575
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58580/
Test 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-219109570
  
Merged build finished. Test 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-219109309
  
**[Test build #58580 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58580/consoleFull)**
 for PR 13061 at commit 
[`a0cfdcc`](https://github.com/apache/spark/commit/a0cfdcc7adfeb08a42d4d4289476e83b5fe7195b).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[tgravescs]

Github user tgravescs commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63207916
  
--- Diff: build/spark-build-info ---
@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# This script generates the build info for spark and places it into the 
spark-version-info.properties file.
--- End diff --

would be nice to document inputs


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[tgravescs]

Github user tgravescs commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63207823
  
--- Diff: build/spark-build-info ---
@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# This script generates the build info for spark and places it into the 
spark-version-info.properties file.
+
+RESOURCE_DIR=$1
+#RESOURCE_DIR=core/target/extra-resources
--- End diff --

remove commented out 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-219085614
  
Jenkins, this is okay to test 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-219082377
  
Both mvn and sbt now call a shell script 
spark/build/spark-build-info which generates the 
spark-build-info.properties file. This is placed under 
core/target/extra-resources and gets packaged in the core jar.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-219082383
  
**[Test build #58580 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58580/consoleFull)**
 for PR 13061 at commit 
[`a0cfdcc`](https://github.com/apache/spark/commit/a0cfdcc7adfeb08a42d4d4289476e83b5fe7195b).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-219081717
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218821484
  
@tgravescs it doesn't necessarily need to be in the classes directory, just 
not in the source tree. The classes directory is the easiest place I know, 
since it doesn't require changing the pom to tell maven where to find it. (I'm 
not sure which plugin's configuration need to be changed either - is it the jar 
or the resources plugin?)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218783441
  
I believe the license header isn't technically needed since its not checked 
in but if its easy to add it doesn't hurt either.

@vanzin  why do you want to put a resource/properties file into the classes 
directory?  
Personally I am not fond of that.   Unfortunately I'm not an maven expert 
but I wonder if we can put it in like target/extra-resources and have it 
automatically picked up still.  
http://maven.apache.org/plugins/maven-resources-plugin/examples/copy-resources.html


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[tgravescs]

Github user tgravescs commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63033687
  
--- Diff: core/pom.xml ---
@@ -335,9 +335,63 @@
   
 
target/scala-${scala.binary.version}/classes
 
target/scala-${scala.binary.version}/test-classes
+
+  
--- End diff --

I believe we don't need this section since it automatically picks up 
anything under the resources directory. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[tgravescs]

Github user tgravescs commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63025127
  
--- Diff: core/src/main/scala/org/apache/spark/package.scala ---
@@ -41,7 +41,39 @@ package org.apache
  * level interfaces. These are subject to changes or removal in minor 
releases.
  */
 
+import java.util.Properties
+
+import org.apache.spark.internal.Logging
+
 package object spark {
-  // For package docs only
-  val SPARK_VERSION = "2.0.0-SNAPSHOT"
+
+  object SparkBuildInfo extends Logging {
+
+val (spark_version: String, spark_branch: String, spark_revision: 
String,
+  spark_build_user: String, spark_repo_url: String, spark_build_date: 
String) =
+  try {
+val unknownProp = ""
+val props = new Properties()
+props.load(Thread.currentThread().getContextClassLoader.
+  getResourceAsStream("spark-version-info.properties"))
--- End diff --

you need to close the input stream that is opened in getResourceAsStream


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[tgravescs]

Github user tgravescs commented on a diff in the pull request:

https://github.com/apache/spark/pull/13061#discussion_r63023786
  
--- Diff: core/src/main/scala/org/apache/spark/package.scala ---
@@ -41,7 +41,39 @@ package org.apache
  * level interfaces. These are subject to changes or removal in minor 
releases.
  */
 
+import java.util.Properties
+
+import org.apache.spark.internal.Logging
+
 package object spark {
-  // For package docs only
-  val SPARK_VERSION = "2.0.0-SNAPSHOT"
+
+  object SparkBuildInfo extends Logging {
+
+val (spark_version: String, spark_branch: String, spark_revision: 
String,
+  spark_build_user: String, spark_repo_url: String, spark_build_date: 
String) =
+  try {
+val unknownProp = ""
+val props = new Properties()
+props.load(Thread.currentThread().getContextClassLoader.
+  getResourceAsStream("spark-version-info.properties"))
+(
+  props.getProperty("version", unknownProp),
+  props.getProperty("branch", unknownProp),
+  props.getProperty("revision", unknownProp),
+  props.getProperty("user", unknownProp),
+  props.getProperty("url", unknownProp),
+  props.getProperty("date", unknownProp)
+)
+  } catch {
+case e: Exception => logError("Unable to read spark build 
properties.", e);
--- End diff --

nit: put the logError on the next 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218634428
  
> I tried running the tests on my local machine and it complained

That's probably because you're currently generating the file in the source 
directory. If you generate it in the build directory, I'm pretty sure that will 
go away.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

Github user dhruve commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218633161
  
Hello @vanzin,

- A common shell script seems to be a good suggestion. 
- I will update the location of the properties file. 

For the apache license header, I tried running the tests on my local 
machine and it complained of the missing header, so I included 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218624682
  
Merged build finished. Test 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218624684
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58413/
Test 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218624533
  
**[Test build #58413 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58413/consoleFull)**
 for PR 13061 at commit 
[`a6a241e`](https://github.com/apache/spark/commit/a6a241eeba025c213e21b1c488a66afa7de75b0f).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218614273
  
Hi @dhruve,

This is going in the right direction; this looks a lot more manageable than 
the previous plugin-based approach. I have a couple of request though:

- Instead of embedding the shell code in the maven and sbt builds, could 
you actually write a shell script and execute it from both places? Then the 
code is shared, easier to read and debug, all that good stuff.

- I don't really like code that pollutes the source directory; you should 
instead generate the properties file in the build directory. That should be 
done in the `generate-resources` phase of the maven build, and I think the 
easiest way is to write the file to `${project.build.directory}/classes` 
directly.

For the sbt build you probably would have to hook something up to 
`resourceGenerators`; there are a couple of examples of using that in the build 
file, you could start from those.

I might be wrong, but I don't believe the generated file needs a copyright 
header.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218603852
  
**[Test build #58413 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58413/consoleFull)**
 for PR 13061 at commit 
[`a6a241e`](https://github.com/apache/spark/commit/a6a241eeba025c213e21b1c488a66afa7de75b0f).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218602776
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/13061#issuecomment-218599104
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-14279] [Build] Pick the spark version f...

[dhruve]

GitHub user dhruve opened a pull request:

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

[SPARK-14279] [Build] Pick the spark version from pom

## What changes were proposed in this pull request?
Change the way spark picks up version information. Also embed the build 
information to better identify the spark version running. 

More context can be found here : https://github.com/apache/spark/pull/12152

## How was this patch tested?
Ran the mvn and sbt builds to verify the version information was being 
displayed correctly on executing spark-submit --version 


![image](https://cloud.githubusercontent.com/assets/7732317/15197251/f7c673a2-1795-11e6-8b2f-88f2a70cf1c1.png)



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

$ git pull https://github.com/dhruve/spark impr/SPARK-14279

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

https://github.com/apache/spark/pull/13061.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 #13061


commit f3d4163f25249c2662cdb56b40ef63001be7d965
Author: Dhruve Ashar 
Date:   2016-05-11T13:33:24Z

Pick Spark Version from pom.xml

commit 813380c9f2ad350c77beaf4f2b93c1818852d7aa
Author: Dhruve Ashar 
Date:   2016-05-11T21:27:39Z

Moved the ant-run task to core/pom.xml




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org