charliec05 commented on code in PR #58401:
URL: https://github.com/apache/spark/pull/58401#discussion_r3885046516
##########
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicDriverFeatureStepSuite.scala:
##########
@@ -404,6 +404,31 @@ class BasicDriverFeatureStepSuite extends SparkFunSuite {
path.startsWith(FILE_UPLOAD_PATH) &&
path.endsWith("some-local-jar.jar")))
}
+ test("SPARK-58969: Local dependency uploads preserve URI fragments") {
+ val fileUploadPath = "s3a://some-bucket/upload-path"
+ val sparkConf = new SparkConf()
+ .set(CONTAINER_IMAGE, "spark-driver:latest")
+ .set(JARS, Seq("/tmp/library-random.jar#library.jar"))
+ .set(FILES, Seq("/tmp/query-random.sql#query.sql"))
+ .set(SUBMIT_PYTHON_FILES, Seq("/tmp/module-random.py#module.py"))
+ .set(KUBERNETES_FILE_UPLOAD_PATH, fileUploadPath)
+ .set("spark.hadoop.fs.s3a.impl",
classOf[TestFileSystem].getCanonicalName)
+ .set("spark.hadoop.fs.s3a.impl.disable.cache", "true")
+ val kubernetesConf = KubernetesTestConf.createDriverConf(sparkConf =
sparkConf)
+
+ val properties = new BasicDriverFeatureStep(kubernetesConf)
+ .getAdditionalPodSystemProperties()
+ Seq(
+ (JARS.key, "library-random.jar", "library.jar"),
+ (FILES.key, "query-random.sql", "query.sql"),
+ (SUBMIT_PYTHON_FILES.key, "module-random.py", "module.py")
+ ).foreach { case (key, physicalName, alias) =>
Review Comment:
Done in 5785b55f. The regression setup and assertion table now include ,
which pins down the existing archive behavior alongside JARs, files, and
py-files.
##########
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicDriverFeatureStepSuite.scala:
##########
@@ -404,6 +404,31 @@ class BasicDriverFeatureStepSuite extends SparkFunSuite {
path.startsWith(FILE_UPLOAD_PATH) &&
path.endsWith("some-local-jar.jar")))
}
+ test("SPARK-58969: Local dependency uploads preserve URI fragments") {
+ val fileUploadPath = "s3a://some-bucket/upload-path"
+ val sparkConf = new SparkConf()
+ .set(CONTAINER_IMAGE, "spark-driver:latest")
+ .set(JARS, Seq("/tmp/library-random.jar#library.jar"))
+ .set(FILES, Seq("/tmp/query-random.sql#query.sql"))
+ .set(SUBMIT_PYTHON_FILES, Seq("/tmp/module-random.py#module.py"))
+ .set(KUBERNETES_FILE_UPLOAD_PATH, fileUploadPath)
+ .set("spark.hadoop.fs.s3a.impl",
classOf[TestFileSystem].getCanonicalName)
+ .set("spark.hadoop.fs.s3a.impl.disable.cache", "true")
+ val kubernetesConf = KubernetesTestConf.createDriverConf(sparkConf =
sparkConf)
+
+ val properties = new BasicDriverFeatureStep(kubernetesConf)
+ .getAdditionalPodSystemProperties()
+ Seq(
+ (JARS.key, "library-random.jar", "library.jar"),
+ (FILES.key, "query-random.sql", "query.sql"),
+ (SUBMIT_PYTHON_FILES.key, "module-random.py", "module.py")
+ ).foreach { case (key, physicalName, alias) =>
+ val uploaded = new java.net.URI(properties(key))
Review Comment:
Done in 5785b55f. I added imports and use the short name.
##########
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/BasicDriverFeatureStepSuite.scala:
##########
@@ -404,6 +404,31 @@ class BasicDriverFeatureStepSuite extends SparkFunSuite {
path.startsWith(FILE_UPLOAD_PATH) &&
path.endsWith("some-local-jar.jar")))
}
+ test("SPARK-58969: Local dependency uploads preserve URI fragments") {
+ val fileUploadPath = "s3a://some-bucket/upload-path"
Review Comment:
Done in 5785b55f. Renamed the value to to match the neighboring test.
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicDriverFeatureStep.scala:
##########
@@ -180,12 +180,8 @@ private[spark] class BasicDriverFeatureStep(conf:
KubernetesDriverConf)
}
val resolved = KubernetesUtils.uploadAndTransformFileUris(value,
Some(conf.sparkConf))
if (resolved.nonEmpty) {
- val resolvedValue = if (key == ARCHIVES) {
- localUris.zip(resolved).map { case (uri, r) =>
- Utils.getUriBuilder(r).fragment(new
java.net.URI(uri).getFragment).build().toString
- }
- } else {
- resolved
+ val resolvedValue = localUris.zip(resolved).map { case (uri, r) =>
+ Utils.getUriBuilder(r).fragment(new
java.net.URI(uri).getFragment).build().toString
}
Review Comment:
Done in 5785b55f. Fragment-less uploaded URIs now return directly; is only
used when the original URI actually had a fragment.
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicDriverFeatureStep.scala:
##########
@@ -180,12 +180,8 @@ private[spark] class BasicDriverFeatureStep(conf:
KubernetesDriverConf)
}
Review Comment:
Done in 5785b55f. Fragment removal is now applied uniformly to all four
local dependency keys, and the original fragment is carried alongside the URI
so removal and restoration stay symmetric.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
