[GitHub] [spark] HyukjinKwon commented on a change in pull request #31877: [SPARK-34783][K8S] Support remote template files

2021-03-18 Thread GitBox 


HyukjinKwon commented on a change in pull request #31877:
URL: https://github.com/apache/spark/pull/31877#discussion_r597336694



##
File path: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/PodTemplateConfigMapStep.scala
##
@@ -75,7 +78,10 @@ private[spark] class PodTemplateConfigMapStep(conf: 
KubernetesConf)
   override def getAdditionalKubernetesResources(): Seq[HasMetadata] = {
 if (hasTemplate) {
   val podTemplateFile = conf.get(KUBERNETES_EXECUTOR_PODTEMPLATE_FILE).get
-  val podTemplateString = Files.toString(new File(podTemplateFile), 
StandardCharsets.UTF_8)
+  val hadoopConf = SparkHadoopUtil.get.newConfiguration(conf.sparkConf)
+  val file = downloadFile(podTemplateFile, Utils.createTempDir(), 
conf.sparkConf, hadoopConf)
+  .stripPrefix("file:").stripPrefix("local:")

Review comment:
   ```suggestion
 val uri = downloadFile(podTemplateFile, Utils.createTempDir(), 
conf.sparkConf, hadoopConf)
 val file = new java.net.URI(uri).getPath
   ```




-- 
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



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

[GitHub] [spark] HyukjinKwon commented on a change in pull request #31877: [SPARK-34783][K8S] Support remote template files

2021-03-18 Thread GitBox 


HyukjinKwon commented on a change in pull request #31877:
URL: https://github.com/apache/spark/pull/31877#discussion_r597336405



##
File path: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
##
@@ -81,9 +82,13 @@ private[spark] object KubernetesUtils extends Logging {
 
   def loadPodFromTemplate(
   kubernetesClient: KubernetesClient,
-  templateFile: File,
-  containerName: Option[String]): SparkPod = {
+  templateFileName: String,
+  containerName: Option[String],
+  conf: SparkConf): SparkPod = {
 try {
+  val hadoopConf = SparkHadoopUtil.get.newConfiguration(conf)
+  val localFile = downloadFile(templateFileName, Utils.createTempDir(), 
conf, hadoopConf)
+  val templateFile = new 
File(localFile.stripPrefix("file:").stripPrefix("local:"))

Review comment:
   I suggested this because:
   
   - It's already being used, for example, in `addJarToClasspath`. I actually 
used this way when I handled archive too.
   - `downloadFile(...)` returns always a file so should be safe.
   - In this way, we're safe about encoded chars from URI




-- 
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



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

[GitHub] [spark] HyukjinKwon commented on a change in pull request #31877: [SPARK-34783][K8S] Support remote template files

2021-03-18 Thread GitBox 


HyukjinKwon commented on a change in pull request #31877:
URL: https://github.com/apache/spark/pull/31877#discussion_r597336087



##
File path: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
##
@@ -81,9 +82,13 @@ private[spark] object KubernetesUtils extends Logging {
 
   def loadPodFromTemplate(
   kubernetesClient: KubernetesClient,
-  templateFile: File,
-  containerName: Option[String]): SparkPod = {
+  templateFileName: String,
+  containerName: Option[String],
+  conf: SparkConf): SparkPod = {
 try {
+  val hadoopConf = SparkHadoopUtil.get.newConfiguration(conf)
+  val localFile = downloadFile(templateFileName, Utils.createTempDir(), 
conf, hadoopConf)
+  val templateFile = new 
File(localFile.stripPrefix("file:").stripPrefix("local:"))

Review comment:
   ```suggestion
 val localFile = downloadFile(templateFileName, Utils.createTempDir(), 
conf, hadoopConf)
 val templateFile = new File(new java.net.URI(localFile).getPath)
```




-- 
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



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

[GitHub] [spark] HyukjinKwon commented on a change in pull request #31877: [SPARK-34783][K8S] Support remote template files

2021-03-18 Thread GitBox 


HyukjinKwon commented on a change in pull request #31877:
URL: https://github.com/apache/spark/pull/31877#discussion_r597335360



##
File path: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
##
@@ -81,9 +82,13 @@ private[spark] object KubernetesUtils extends Logging {
 
   def loadPodFromTemplate(
   kubernetesClient: KubernetesClient,
-  templateFile: File,
-  containerName: Option[String]): SparkPod = {
+  templateFileName: String,
+  containerName: Option[String],
+  conf: SparkConf): SparkPod = {
 try {
+  val hadoopConf = SparkHadoopUtil.get.newConfiguration(conf)
+  val localFile = downloadFile(templateFileName, Utils.createTempDir(), 
conf, hadoopConf)
+  val templateFile = new 
File(localFile.stripPrefix("file:").stripPrefix("local:"))

Review comment:
   Oh




-- 
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



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

[GitHub] [spark] HyukjinKwon commented on a change in pull request #31877: [SPARK-34783][K8S] Support remote template files

2021-03-17 Thread GitBox 


HyukjinKwon commented on a change in pull request #31877:
URL: https://github.com/apache/spark/pull/31877#discussion_r596554864



##
File path: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
##
@@ -79,12 +79,25 @@ private[spark] object KubernetesUtils extends Logging {
 opt2.foreach { _ => require(opt1.isEmpty, errMessage) }
   }
 
+  def getLocalTemplateFile(path: String, conf: SparkConf): File = {

Review comment:
   @dongjoon-hyun how about leveraging utils such as 
`DependencyUtils.downloadFile` that is used for Spark submit?





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



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