charliec05 commented on code in PR #58401:
URL: https://github.com/apache/spark/pull/58401#discussion_r3885046508
##########
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 `r` directly;
`UriBuilder` is only used when the original URI actually had a fragment.
##########
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 `java.net.URI` imports and use the short `URI`
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"
+ 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
`ARCHIVES`, 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"
Review Comment:
Done in 5785b55f. Renamed the value to `FILE_UPLOAD_PATH` to match the
neighboring 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: [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]
