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 2022/01/30 03:49:36 UTC
[GitHub] [spark] williamhyun opened a new pull request #35364: [SPARK-38071][K8S][TESTS] Support K8s namespace parameter in SBT K8s IT
williamhyun opened a new pull request #35364:
URL: https://github.com/apache/spark/pull/35364
### What changes were proposed in this pull request?
This PR aims to support K8s namespace parameter in SBT K8s integration test.
### Why are the changes needed?
- This allows the users to set the test namespace name
- When there is no given namespace, it will generate a random namespace and use it like `Maven` test
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Manually using the following command
```
build/sbt -Psparkr -Pkubernetes -Pkubernetes-integration-tests -Dtest.exclude.tags=minikube -Dspark.kubernetes.test.deployMode=docker-for-desktop -Dspark.kubernetes.test.namespace=spark-it-test "kubernetes-integration-tests/test"
```
--
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] dongjoon-hyun closed pull request #35364: [SPARK-38071][K8S][TESTS] Support K8s namespace parameter in SBT K8s IT
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #35364:
URL: https://github.com/apache/spark/pull/35364
--
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] martin-g commented on a change in pull request #35364: [SPARK-38071][K8S][TESTS] Support K8s namespace parameter in SBT K8s IT
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #35364:
URL: https://github.com/apache/spark/pull/35364#discussion_r818418579
##########
File path: project/SparkBuild.scala
##########
@@ -671,9 +670,9 @@ object KubernetesIntegrationTests {
(Test / javaOptions) ++= Seq(
s"-Dspark.kubernetes.test.deployMode=${deployMode.getOrElse("minikube")}",
s"-Dspark.kubernetes.test.imageTag=${imageTag.value}",
- s"-Dspark.kubernetes.test.namespace=${namespace.value}",
s"-Dspark.kubernetes.test.unpackSparkDir=$sparkHome"
),
+ (Test / javaOptions) ++= namespace.map("-Dspark.kubernetes.test.namespace=" + _),
Review comment:
Shouldn't this be: `(Test / javaOptions) += namespace.map("-Dspark.kubernetes.test.namespace=" + _).getOrElse("default"),` ?
The old `default` value from line 637 seems to be lost now.
--
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] martin-g commented on a change in pull request #35364: [SPARK-38071][K8S][TESTS] Support K8s namespace parameter in SBT K8s IT
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #35364:
URL: https://github.com/apache/spark/pull/35364#discussion_r818492540
##########
File path: project/SparkBuild.scala
##########
@@ -671,9 +670,9 @@ object KubernetesIntegrationTests {
(Test / javaOptions) ++= Seq(
s"-Dspark.kubernetes.test.deployMode=${deployMode.getOrElse("minikube")}",
s"-Dspark.kubernetes.test.imageTag=${imageTag.value}",
- s"-Dspark.kubernetes.test.namespace=${namespace.value}",
s"-Dspark.kubernetes.test.unpackSparkDir=$sparkHome"
),
+ (Test / javaOptions) ++= namespace.map("-Dspark.kubernetes.test.namespace=" + _),
Review comment:
Thanks for explaining, @dongjoon-hyun !
--
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] dongjoon-hyun commented on pull request #35364: [SPARK-38071][K8S][TESTS] Support K8s namespace parameter in SBT K8s IT
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #35364:
URL: https://github.com/apache/spark/pull/35364#issuecomment-1057270274
I backported this test-only patch to `branch-3.2` for Apache Spark 3.2.2. Thank you, @williamhyun .
--
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] dongjoon-hyun commented on a change in pull request #35364: [SPARK-38071][K8S][TESTS] Support K8s namespace parameter in SBT K8s IT
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #35364:
URL: https://github.com/apache/spark/pull/35364#discussion_r818455216
##########
File path: project/SparkBuild.scala
##########
@@ -671,9 +670,9 @@ object KubernetesIntegrationTests {
(Test / javaOptions) ++= Seq(
s"-Dspark.kubernetes.test.deployMode=${deployMode.getOrElse("minikube")}",
s"-Dspark.kubernetes.test.imageTag=${imageTag.value}",
- s"-Dspark.kubernetes.test.namespace=${namespace.value}",
s"-Dspark.kubernetes.test.unpackSparkDir=$sparkHome"
),
+ (Test / javaOptions) ++= namespace.map("-Dspark.kubernetes.test.namespace=" + _),
Review comment:
This change was intentional, @martin-g .
- This new default is consistent with `Maven` integration test's default test coverage which uses random namespaces.
- Last time, we missed the namespace propagation issue because SBT only tested with a single `default` namespace at that time. I fixed that later via ed3ea989f97634374789d173f1cc932230bf3aa1 .
After this change, we have the same test coverage and behavior in Maven/SBT.
--
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