You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/09/16 04:48:34 UTC
[GitHub] [spark] JoshRosen opened a new pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
JoshRosen opened a new pull request #34013:
URL: https://github.com/apache/spark/pull/34013
### What changes were proposed in this pull request?
This PR refactors test code in order to improve the debugability of `SparkSubmitSuite`.
The `sql/hive` module contains a `SparkSubmitTestUtils` helper class which launches `spark-submit` and captures its output in order to display better error messages when tests fail. This helper is currently used by `HiveSparkSubmitSuite` and `HiveExternalCatalogVersionsSuite`, but isn't used by `SparkSubmitSuite`.
In this PR, I moved `SparkSubmitTestUtils` and `ProcessTestUtils` into the `core` module and updated `SparkSubmitSuite`, `BufferHolderSparkSubmitSuite`, and `WholestageCodegenSparkSubmitSuite` to use the relocated helper classes. This required me to change `SparkSubmitTestUtils` to make its timeouts configurable and to generalize its method for locating the `spark-submit` binary.
### Why are the changes needed?
Previously, `SparkSubmitSuite` tests would fail with messages like:
```
Process returned with exit code 1. See the log4j logs for more detail.
```
which require the Spark developer to hunt in log4j logs in order to view the logs from the failed `spark-submit` command.
After this change, those tests will fail with detailed error messages that include the text of failed command plus timestamped logs captured from the failed process.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
I manually ran the affected test suites.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen removed a comment on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
JoshRosen removed a comment on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920572637
Here's an example of the new output (I didn't include it in the PR description because it's so big):
```
[info] - temporary Hive UDF: define a UDF and use it *** FAILED *** (4 seconds, 135 milliseconds)
[info] spark-submit returned with exit code 101.
[info] Command line: '/Users/joshrosen/oss-spark/bin/spark-submit' '--class' 'wrongClassName' '--name' 'TemporaryHiveUDFTest' '--master' 'local-cluster[2,1,1024]' '--conf' 'spark.ui.enabled=false' '--conf' 'spark.master.rest.enabled=false' '--driver-java-options' '-Dderby.system.durability=test' '--jars' 'file:/Users/joshrosen/oss-spark/target/tmp/spark-32d0a47c-33eb-4488-866a-9994b3727b5b/testJar-1631766233448.jar,file:/Users/joshrosen/oss-spark/target/tmp/spark-e9e32588-83fa-43bd-a60f-9538fd30ab9a/testJar-1631766233601.jar' 'file:/Users/joshrosen/oss-spark/target/tmp/spark-daef4628-d953-41e3-b935-a726a7796418/testJar-1631766232701.jar' 'SparkSubmitClassA' 'SparkSubmitClassB'
[info]
[info] 2021-09-15 21:23:54.752 - stderr> SLF4J: Class path contains multiple SLF4J bindings.
[info] 2021-09-15 21:23:54.752 - stderr> SLF4J: Found binding in [jar:file:/Users/joshrosen/oss-spark/assembly/target/scala-2.12/jars/slf4j-log4j12-1.7.30.jar!/org/slf4j/impl/StaticLoggerBinder.class]
[info] 2021-09-15 21:23:54.752 - stderr> SLF4J: Found binding in [jar:file:/Users/joshrosen/.m2/repository/org/slf4j/slf4j-log4j12/1.7.30/slf4j-log4j12-1.7.30.jar!/org/slf4j/impl/StaticLoggerBinder.class]
[info] 2021-09-15 21:23:54.752 - stderr> SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
[info] 2021-09-15 21:23:54.753 - stderr> SLF4J: Actual binding is of type [org.slf4j.impl.Log4jLoggerFactory]
[info] 2021-09-15 21:23:55.623 - stderr> Error: Failed to load class wrongClassName. (SparkSubmitTestUtils.scala:97)
[info] org.scalatest.exceptions.TestFailedException:
[info] at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
[info] at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471)
[info] at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1563)
[info] at org.scalatest.Assertions.fail(Assertions.scala:933)
[info] at org.scalatest.Assertions.fail$(Assertions.scala:929)
[info] at org.scalatest.funsuite.AnyFunSuite.fail(AnyFunSuite.scala:1563)
[info] at org.apache.spark.deploy.SparkSubmitTestUtils.runSparkSubmit(SparkSubmitTestUtils.scala:97)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920597396
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47835/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen closed pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
JoshRosen closed pull request #34013:
URL: https://github.com/apache/spark/pull/34013
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920752285
**[Test build #143340 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/143340/testReport)** for PR 34013 at commit [`5ad5c38`](https://github.com/apache/spark/commit/5ad5c38c4f758de38d335cada58425aeed224281).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920673599
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47847/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920753913
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/143340/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920673599
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47847/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen commented on a change in pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
JoshRosen commented on a change in pull request #34013:
URL: https://github.com/apache/spark/pull/34013#discussion_r709823439
##########
File path: core/src/test/scala/org/apache/spark/deploy/SparkSubmitTestUtils.scala
##########
@@ -25,33 +25,36 @@ import scala.collection.mutable.ArrayBuffer
import org.scalatest.concurrent.{Signaler, ThreadSignaler, TimeLimits}
import org.scalatest.exceptions.TestFailedDueToTimeoutException
+import org.scalatest.time.Span
import org.scalatest.time.SpanSugar._
+import org.apache.spark.ProcessTestUtils.ProcessOutputCapturer
import org.apache.spark.SparkFunSuite
-import org.apache.spark.sql.test.ProcessTestUtils.ProcessOutputCapturer
import org.apache.spark.util.Utils
trait SparkSubmitTestUtils extends SparkFunSuite with TimeLimits {
+ protected val defaultSparkSubmitTimeout: Span = 1.minute
+
// Necessary to make ScalaTest 3.x interrupt a thread on the JVM like ScalaTest 2.2.x
implicit val defaultSignaler: Signaler = ThreadSignaler
// NOTE: This is an expensive operation in terms of time (10 seconds+). Use sparingly.
- // This is copied from org.apache.spark.deploy.SparkSubmitSuite
protected def runSparkSubmit(
args: Seq[String],
sparkHomeOpt: Option[String] = None,
+ timeout: Span = defaultSparkSubmitTimeout,
isSparkTesting: Boolean = true): Unit = {
val sparkHome = sparkHomeOpt.getOrElse(
sys.props.getOrElse("spark.test.home", fail("spark.test.home is not set!")))
val history = ArrayBuffer.empty[String]
val sparkSubmit = if (Utils.isWindows) {
// On Windows, `ProcessBuilder.directory` does not change the current working directory.
- new File("..\\..\\bin\\spark-submit.cmd").getAbsolutePath
+ new File(new File(sparkHome, "bin"), "spark-submit.cmd")
} else {
- "./bin/spark-submit"
+ new File(new File(sparkHome, "bin"), "spark-submit")
}
- val commands = Seq(sparkSubmit) ++ args
+ val commands = Seq(sparkSubmit.getCanonicalPath) ++ args
Review comment:
Here's my reasoning on why this test-utility-only behavior change is okay:
1. [`SparkSubmitSuite`'s version of `runSparkSubmit`](https://github.com/apache/spark/blob/afd406e4d0b9701d3733ac8afe44339661fb4853/core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala#L1536-L1544) was already calling `getCanonicalPath`.
In `getCanonicalPath`, paths are resolved relative to the test JVM's current working directory, whose depth from `SPARK_HOME` differs across submodules (it's `../` in `core` vs `../../` in `sql/hive`). If I wanted to preserve this path resolution strategy then I'd need to add a way to customize the `$root` prefix in different suites and that feels error-prone and clunky. It's much simpler to just specify a path relative to `sparkHome`.
Given this, I think this change is correct w.r.t. matching the `SparkSubmitSuite` behavior.
2. The original `SparkSubmitTestUtils` didn't use canonical paths. Instead, it relied on being able to change the process builder's working directory so that the process builder would correctly resolve the command's relative path. In an old PR, it looks like @HyukjinKwon originally considered using canonical or absolute paths but stuck with the relative paths out of concern that the change might (possibly) have caused `HiveSparkSubmitSuite` to become flaky: https://github.com/apache/spark/pull/16451/files#diff-b71d15c52a3d16c9e69cb230aefe4e0ef1a00eb459ee84e08f9e63f39ddf3525. If we don't see any new evidence of flakiness then I think we should proceed with unifying on this PR's simpler approach.
3. [HiveExternalCatalogVersionsSuite calls `runSparkSubmit` with a `sparkHome` pointing to a downloaded old version of Spark](https://github.com/apache/spark/blob/c21779729765f640272e0249c6e71145aa5ccfdf/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala#L219). The purpose of that code is to create testing warehouses with old versions of Spark, so I believe the intent is to invoke the version of `spark-submit` bundled in those versions. With the old version of this code, the behavior would differ across OS's: windows builds would invoke the version of `spark-submit` in the Spark repository (because the process builder working directory would be ignored), whereas non-windows builds would invoke the `spark-submit` in the downloaded Spark. Despite this, things still worked correctly because the `spark-submit` script [invokes the copy of `spark-class` found in `$SPARK_HOME`](https://github.com/apache/spark/blob/c21779729765f640272e0249c6e71145aa5
ccfdf/bin/spark-submit#L27). That's a bit confusing, though, so I think it's better to just use absolute paths as I've done here.
I'm going to remove the
```
// On Windows, `ProcessBuilder.directory` does not change the current working directory.
```
comment since I think it's now superfluous.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
JoshRosen commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920572637
Here's an example of the new output (I didn't include it in the PR description because it's so big):
```
[info] - temporary Hive UDF: define a UDF and use it *** FAILED *** (4 seconds, 135 milliseconds)
[info] spark-submit returned with exit code 101.
[info] Command line: '/Users/joshrosen/oss-spark/bin/spark-submit' '--class' 'wrongClassName' '--name' 'TemporaryHiveUDFTest' '--master' 'local-cluster[2,1,1024]' '--conf' 'spark.ui.enabled=false' '--conf' 'spark.master.rest.enabled=false' '--driver-java-options' '-Dderby.system.durability=test' '--jars' 'file:/Users/joshrosen/oss-spark/target/tmp/spark-32d0a47c-33eb-4488-866a-9994b3727b5b/testJar-1631766233448.jar,file:/Users/joshrosen/oss-spark/target/tmp/spark-e9e32588-83fa-43bd-a60f-9538fd30ab9a/testJar-1631766233601.jar' 'file:/Users/joshrosen/oss-spark/target/tmp/spark-daef4628-d953-41e3-b935-a726a7796418/testJar-1631766232701.jar' 'SparkSubmitClassA' 'SparkSubmitClassB'
[info]
[info] 2021-09-15 21:23:54.752 - stderr> SLF4J: Class path contains multiple SLF4J bindings.
[info] 2021-09-15 21:23:54.752 - stderr> SLF4J: Found binding in [jar:file:/Users/joshrosen/oss-spark/assembly/target/scala-2.12/jars/slf4j-log4j12-1.7.30.jar!/org/slf4j/impl/StaticLoggerBinder.class]
[info] 2021-09-15 21:23:54.752 - stderr> SLF4J: Found binding in [jar:file:/Users/joshrosen/.m2/repository/org/slf4j/slf4j-log4j12/1.7.30/slf4j-log4j12-1.7.30.jar!/org/slf4j/impl/StaticLoggerBinder.class]
[info] 2021-09-15 21:23:54.752 - stderr> SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
[info] 2021-09-15 21:23:54.753 - stderr> SLF4J: Actual binding is of type [org.slf4j.impl.Log4jLoggerFactory]
[info] 2021-09-15 21:23:55.623 - stderr> Error: Failed to load class wrongClassName. (SparkSubmitTestUtils.scala:97)
[info] org.scalatest.exceptions.TestFailedException:
[info] at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
[info] at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471)
[info] at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1563)
[info] at org.scalatest.Assertions.fail(Assertions.scala:933)
[info] at org.scalatest.Assertions.fail$(Assertions.scala:929)
[info] at org.scalatest.funsuite.AnyFunSuite.fail(AnyFunSuite.scala:1563)
[info] at org.apache.spark.deploy.SparkSubmitTestUtils.runSparkSubmit(SparkSubmitTestUtils.scala:97)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920625848
**[Test build #143340 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/143340/testReport)** for PR 34013 at commit [`5ad5c38`](https://github.com/apache/spark/commit/5ad5c38c4f758de38d335cada58425aeed224281).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen commented on a change in pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
JoshRosen commented on a change in pull request #34013:
URL: https://github.com/apache/spark/pull/34013#discussion_r709823439
##########
File path: core/src/test/scala/org/apache/spark/deploy/SparkSubmitTestUtils.scala
##########
@@ -25,33 +25,36 @@ import scala.collection.mutable.ArrayBuffer
import org.scalatest.concurrent.{Signaler, ThreadSignaler, TimeLimits}
import org.scalatest.exceptions.TestFailedDueToTimeoutException
+import org.scalatest.time.Span
import org.scalatest.time.SpanSugar._
+import org.apache.spark.ProcessTestUtils.ProcessOutputCapturer
import org.apache.spark.SparkFunSuite
-import org.apache.spark.sql.test.ProcessTestUtils.ProcessOutputCapturer
import org.apache.spark.util.Utils
trait SparkSubmitTestUtils extends SparkFunSuite with TimeLimits {
+ protected val defaultSparkSubmitTimeout: Span = 1.minute
+
// Necessary to make ScalaTest 3.x interrupt a thread on the JVM like ScalaTest 2.2.x
implicit val defaultSignaler: Signaler = ThreadSignaler
// NOTE: This is an expensive operation in terms of time (10 seconds+). Use sparingly.
- // This is copied from org.apache.spark.deploy.SparkSubmitSuite
protected def runSparkSubmit(
args: Seq[String],
sparkHomeOpt: Option[String] = None,
+ timeout: Span = defaultSparkSubmitTimeout,
isSparkTesting: Boolean = true): Unit = {
val sparkHome = sparkHomeOpt.getOrElse(
sys.props.getOrElse("spark.test.home", fail("spark.test.home is not set!")))
val history = ArrayBuffer.empty[String]
val sparkSubmit = if (Utils.isWindows) {
// On Windows, `ProcessBuilder.directory` does not change the current working directory.
- new File("..\\..\\bin\\spark-submit.cmd").getAbsolutePath
+ new File(new File(sparkHome, "bin"), "spark-submit.cmd")
} else {
- "./bin/spark-submit"
+ new File(new File(sparkHome, "bin"), "spark-submit")
}
- val commands = Seq(sparkSubmit) ++ args
+ val commands = Seq(sparkSubmit.getCanonicalPath) ++ args
Review comment:
Here's my reasoning on why this test-utility-only behavior change is okay:
1. [`SparkSubmitSuite`'s version of `runSparkSubmit`](https://github.com/apache/spark/blob/afd406e4d0b9701d3733ac8afe44339661fb4853/core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala#L1536-L1544) was already calling `getCanonicalPath`.
In `getCanonicalPath`, paths are resolved relative to the test JVM's current working directory, whose depth from `SPARK_HOME` differs across submodules (it's `../` in `core` vs `../../` in `sql/hive`). If I wanted to preserve this path resolution strategy then I'd need to add a way to customize the `$root` prefix in different suites and that feels error-prone and clunky. It's much simpler to just specify a path relative to `sparkHome`.
Given this, I think this change is correct w.r.t. matching the `SparkSubmitSuite` behavior.
2. The original `SparkSubmitTestUtils` didn't use canonical paths. Instead, it relied on being able to change the process builder's working directory. In an old PR, it looks like @HyukjinKwon originally considered using canonical or absolute paths but stuck with the relative paths out of concern that the change might (possibly) have caused `HiveSparkSubmitSuite` to become flaky: https://github.com/apache/spark/pull/16451/files#diff-b71d15c52a3d16c9e69cb230aefe4e0ef1a00eb459ee84e08f9e63f39ddf3525. If we don't see any new evidence of flakiness then I think we should proceed with unifying on this PR's simpler approach.
3. [HiveExternalCatalogVersionsSuite calls `runSparkSubmit` with a `sparkHome` pointing to a downloaded old version of Spark](https://github.com/apache/spark/blob/c21779729765f640272e0249c6e71145aa5ccfdf/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala#L219). The purpose of that code is to create testing warehouses with old versions of Spark, so I believe the intent is to invoke the version of `spark-submit` bundled in those versions. With the old version of this code, the behavior would differ across OS's: windows builds would invoke the version of `spark-submit` in the Spark repository (because the process builder working directory would be ignored), whereas non-windows builds would invoke the `spark-submit` in the downloaded Spark. Despite this, things still worked correctly because the `spark-submit` script [invokes the copy of `spark-class` found in `$SPARK_HOME`](https://github.com/apache/spark/blob/c21779729765f640272e0249c6e71145aa5
ccfdf/bin/spark-submit#L27). That's a bit confusing, though, so I think it's better to just use absolute paths as I've done here.
I'm going to remove the
```
// On Windows, `ProcessBuilder.directory` does not change the current working directory.
```
comment since I think it's now superfluous.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920598147
**[Test build #143335 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/143335/testReport)** for PR 34013 at commit [`aa2a485`](https://github.com/apache/spark/commit/aa2a4856dc321c0017b69e922834ac1f2226c79e).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920598147
**[Test build #143335 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/143335/testReport)** for PR 34013 at commit [`aa2a485`](https://github.com/apache/spark/commit/aa2a4856dc321c0017b69e922834ac1f2226c79e).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920597396
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47835/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920573975
**[Test build #143330 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/143330/testReport)** for PR 34013 at commit [`1cb9cef`](https://github.com/apache/spark/commit/1cb9cef605cfb04a98d0a738fe1644e7d31afaba).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920603780
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47839/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920607870
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47839/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920717944
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/143335/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920592617
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47835/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920573148
**[Test build #143330 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/143330/testReport)** for PR 34013 at commit [`1cb9cef`](https://github.com/apache/spark/commit/1cb9cef605cfb04a98d0a738fe1644e7d31afaba).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920716150
**[Test build #143335 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/143335/testReport)** for PR 34013 at commit [`aa2a485`](https://github.com/apache/spark/commit/aa2a4856dc321c0017b69e922834ac1f2226c79e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen commented on a change in pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
JoshRosen commented on a change in pull request #34013:
URL: https://github.com/apache/spark/pull/34013#discussion_r709823439
##########
File path: core/src/test/scala/org/apache/spark/deploy/SparkSubmitTestUtils.scala
##########
@@ -25,33 +25,36 @@ import scala.collection.mutable.ArrayBuffer
import org.scalatest.concurrent.{Signaler, ThreadSignaler, TimeLimits}
import org.scalatest.exceptions.TestFailedDueToTimeoutException
+import org.scalatest.time.Span
import org.scalatest.time.SpanSugar._
+import org.apache.spark.ProcessTestUtils.ProcessOutputCapturer
import org.apache.spark.SparkFunSuite
-import org.apache.spark.sql.test.ProcessTestUtils.ProcessOutputCapturer
import org.apache.spark.util.Utils
trait SparkSubmitTestUtils extends SparkFunSuite with TimeLimits {
+ protected val defaultSparkSubmitTimeout: Span = 1.minute
+
// Necessary to make ScalaTest 3.x interrupt a thread on the JVM like ScalaTest 2.2.x
implicit val defaultSignaler: Signaler = ThreadSignaler
// NOTE: This is an expensive operation in terms of time (10 seconds+). Use sparingly.
- // This is copied from org.apache.spark.deploy.SparkSubmitSuite
protected def runSparkSubmit(
args: Seq[String],
sparkHomeOpt: Option[String] = None,
+ timeout: Span = defaultSparkSubmitTimeout,
isSparkTesting: Boolean = true): Unit = {
val sparkHome = sparkHomeOpt.getOrElse(
sys.props.getOrElse("spark.test.home", fail("spark.test.home is not set!")))
val history = ArrayBuffer.empty[String]
val sparkSubmit = if (Utils.isWindows) {
// On Windows, `ProcessBuilder.directory` does not change the current working directory.
- new File("..\\..\\bin\\spark-submit.cmd").getAbsolutePath
+ new File(new File(sparkHome, "bin"), "spark-submit.cmd")
} else {
- "./bin/spark-submit"
+ new File(new File(sparkHome, "bin"), "spark-submit")
}
- val commands = Seq(sparkSubmit) ++ args
+ val commands = Seq(sparkSubmit.getCanonicalPath) ++ args
Review comment:
Here's my reasoning on why this test-utility-only behavior change is okay:
1. [`SparkSubmitSuite`'s version of `runSparkSubmit`](https://github.com/apache/spark/blob/afd406e4d0b9701d3733ac8afe44339661fb4853/core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala#L1536-L1544) was already calling `getCanonicalPath`.
In `getCanonicalPath`, paths are resolved relative to the test JVM's current working directory, whose depth from `SPARK_HOME` differs across submodules (it's `../` in `core` vs `../../` in `sql/hive`). If I wanted to preserve the old code's path resolution strategy then I'd need to add a way to customize the `$root` prefix in different suites and that feels error-prone and clunky. It's much simpler to just specify a path relative to `sparkHome`.
Given this, I think this change is correct w.r.t. matching the `SparkSubmitSuite` behavior.
2. The original `SparkSubmitTestUtils` didn't use canonical paths. Instead, it relied on being able to change the process builder's working directory so that the process builder would correctly resolve the command's relative path. In an old PR, it looks like @HyukjinKwon originally considered using canonical or absolute paths but stuck with the relative paths out of concern that the change might (possibly) have caused `HiveSparkSubmitSuite` to become flaky: https://github.com/apache/spark/pull/16451/files#diff-b71d15c52a3d16c9e69cb230aefe4e0ef1a00eb459ee84e08f9e63f39ddf3525. If we don't see any new evidence of flakiness then I think we should proceed with unifying on this PR's simpler approach.
3. [HiveExternalCatalogVersionsSuite calls `runSparkSubmit` with a `sparkHome` pointing to a downloaded old version of Spark](https://github.com/apache/spark/blob/c21779729765f640272e0249c6e71145aa5ccfdf/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveExternalCatalogVersionsSuite.scala#L219). The purpose of that code is to create testing warehouses with old versions of Spark, so I believe the intent is to invoke the version of `spark-submit` bundled in those versions. With the old version of this code, the behavior would differ across OS's: windows builds would invoke the version of `spark-submit` in the Spark repository (because the process builder working directory would be ignored), whereas non-windows builds would invoke the `spark-submit` in the downloaded Spark. Despite this, things still worked correctly because the `spark-submit` script [invokes the copy of `spark-class` found in `$SPARK_HOME`](https://github.com/apache/spark/blob/c21779729765f640272e0249c6e71145aa5
ccfdf/bin/spark-submit#L27). That's a bit confusing, though, so I think it's better to just use absolute paths as I've done here.
I'm going to remove the
```
// On Windows, `ProcessBuilder.directory` does not change the current working directory.
```
comment since I think it's now superfluous.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920717944
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/143335/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920607870
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/47839/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920573999
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/143330/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920607837
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47839/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mridulm commented on a change in pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
mridulm commented on a change in pull request #34013:
URL: https://github.com/apache/spark/pull/34013#discussion_r710316154
##########
File path: core/src/test/scala/org/apache/spark/deploy/SparkSubmitTestUtils.scala
##########
@@ -25,33 +25,36 @@ import scala.collection.mutable.ArrayBuffer
import org.scalatest.concurrent.{Signaler, ThreadSignaler, TimeLimits}
import org.scalatest.exceptions.TestFailedDueToTimeoutException
+import org.scalatest.time.Span
import org.scalatest.time.SpanSugar._
+import org.apache.spark.ProcessTestUtils.ProcessOutputCapturer
import org.apache.spark.SparkFunSuite
-import org.apache.spark.sql.test.ProcessTestUtils.ProcessOutputCapturer
import org.apache.spark.util.Utils
trait SparkSubmitTestUtils extends SparkFunSuite with TimeLimits {
+ protected val defaultSparkSubmitTimeout: Span = 1.minute
+
// Necessary to make ScalaTest 3.x interrupt a thread on the JVM like ScalaTest 2.2.x
implicit val defaultSignaler: Signaler = ThreadSignaler
// NOTE: This is an expensive operation in terms of time (10 seconds+). Use sparingly.
- // This is copied from org.apache.spark.deploy.SparkSubmitSuite
protected def runSparkSubmit(
args: Seq[String],
sparkHomeOpt: Option[String] = None,
+ timeout: Span = defaultSparkSubmitTimeout,
isSparkTesting: Boolean = true): Unit = {
val sparkHome = sparkHomeOpt.getOrElse(
sys.props.getOrElse("spark.test.home", fail("spark.test.home is not set!")))
val history = ArrayBuffer.empty[String]
val sparkSubmit = if (Utils.isWindows) {
// On Windows, `ProcessBuilder.directory` does not change the current working directory.
- new File("..\\..\\bin\\spark-submit.cmd").getAbsolutePath
+ new File(new File(sparkHome, "bin"), "spark-submit.cmd")
} else {
- "./bin/spark-submit"
+ new File(new File(sparkHome, "bin"), "spark-submit")
}
- val commands = Seq(sparkSubmit) ++ args
+ val commands = Seq(sparkSubmit.getCanonicalPath) ++ args
Review comment:
Yes, this change looks good to me - it was a cleaner way to handle the paths.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920753913
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/143340/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920667914
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47847/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920573999
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/143330/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920573148
**[Test build #143330 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/143330/testReport)** for PR 34013 at commit [`1cb9cef`](https://github.com/apache/spark/commit/1cb9cef605cfb04a98d0a738fe1644e7d31afaba).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] JoshRosen commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
JoshRosen commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-921259448
Merging to master and branch-3.2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920625848
**[Test build #143340 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/143340/testReport)** for PR 34013 at commit [`5ad5c38`](https://github.com/apache/spark/commit/5ad5c38c4f758de38d335cada58425aeed224281).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920673576
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47847/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #34013: [SPARK-36774][CORE][TESTS] Move SparkSubmitTestUtils to core module and use it in SparkSubmitSuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34013:
URL: https://github.com/apache/spark/pull/34013#issuecomment-920595775
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47835/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org