[GitHub] spark pull request #22820: [SPARK-25828][K8S][BUILD] Bumping Kubernetes-Clie...
Github user erikerlandson commented on a diff in the pull request:
https://github.com/apache/spark/pull/22820#discussion_r228321586
--- Diff:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
---
@@ -157,7 +157,10 @@ private[spark] object KubernetesUtils {
}.getOrElse(Seq(("container state", "N/A")))
}
- def formatTime(time: Time): String = {
-if (time != null) time.getTime else "N/A"
+ def formatTime(time: String): String = {
--- End diff --
I would keep it, but possibly add a comment about data type change
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22820: [SPARK-25828][K8S][BUILD] Bumping Kubernetes-Clie...
Github user erikerlandson commented on a diff in the pull request: https://github.com/apache/spark/pull/22820#discussion_r228321407 --- Diff: docs/running-on-kubernetes.md --- @@ -45,7 +45,8 @@ logs and remains in "completed" state in the Kubernetes API until it's eventuall Note that in the completed state, the driver pod does *not* use any computational or memory resources. -The driver and executor pod scheduling is handled by Kubernetes. It is possible to schedule the +The driver and executor pod scheduling is handled by Kubernetes. Communication to the Kubernetes API is done via fabric8, and we are +currently running kubernetes-client version 4.1.0. Make sure that any infrastructure additions are weary of said version. It is possible to schedule the --- End diff -- nit: "wary" ? or "aware of " ? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22820: [SPARK-25828][K8S][BUILD] Bumping Kubernetes-Clie...
Github user ifilonenko commented on a diff in the pull request:
https://github.com/apache/spark/pull/22820#discussion_r228269911
--- Diff:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
---
@@ -157,7 +157,10 @@ private[spark] object KubernetesUtils {
}.getOrElse(Seq(("container state", "N/A")))
}
- def formatTime(time: Time): String = {
-if (time != null) time.getTime else "N/A"
+ def formatTime(time: String): String = {
--- End diff --
The `time.getTime` is now a String. I wanted to keep the functionality, I
would remove if you deem it to be unnecessary.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22820: [SPARK-25828][K8S][BUILD] Bumping Kubernetes-Clie...
Github user liyinan926 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22820#discussion_r228030940
--- Diff:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/MountVolumesFeatureStep.scala
---
@@ -56,8 +56,10 @@ private[spark] class MountVolumesFeatureStep(
val volumeBuilder = spec.volumeConf match {
case KubernetesHostPathVolumeConf(hostPath) =>
+ // TODO: Backwards compatibility allows for empty string, but
specify
+ // hostPath type here
new VolumeBuilder()
-.withHostPath(new HostPathVolumeSource(hostPath))
+.withHostPath(new HostPathVolumeSource(hostPath, ""))
--- End diff --
Can you add a short descriptive comment after the empty string, e.g., `/*
... */` to explain what the second argument is?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22820: [SPARK-25828][K8S][BUILD] Bumping Kubernetes-Clie...
Github user liyinan926 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22820#discussion_r228030819
--- Diff:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
---
@@ -157,7 +157,10 @@ private[spark] object KubernetesUtils {
}.getOrElse(Seq(("container state", "N/A")))
}
- def formatTime(time: Time): String = {
-if (time != null) time.getTime else "N/A"
+ def formatTime(time: String): String = {
--- End diff --
Why this change?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22820: [SPARK-25828][K8S][BUILD] Bumping Kubernetes-Clie...
Github user skonto commented on a diff in the pull request: https://github.com/apache/spark/pull/22820#discussion_r228005475 --- Diff: resource-managers/kubernetes/core/pom.xml --- @@ -29,7 +29,7 @@ Spark Project Kubernetes kubernetes -3.0.0 +4.0.0 --- End diff -- Yeah just wondering I don't know. What matters is what each version supports. Still no version supports 1.11 https://github.com/fabric8io/kubernetes-client/issues/1235 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
