jzhuge commented on code in PR #59587:
URL: https://github.com/apache/spark/pull/59587#discussion_r4317346240
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala:
##########
@@ -3251,8 +3251,9 @@ class HiveDDLSuite
val udtfJar = TestUDTFJar.jar
val jarName = udtfJar.getName
val jar = udtfJar.toURI.toString
+ // listJars() flattens every artifact bucket, so clear the jar from all
of them.
spark.sparkContext.allAddedJars.keys.find(_.contains(jarName))
- .foreach(k =>
spark.sparkContext.addedJars.get("default").foreach(_.remove(k)))
+ .foreach(k => spark.sparkContext.addedJars.values.foreach(_.remove(k)))
Review Comment:
Agreed. With the unique jar the test no longer touches `addedJars` at all,
so other sessions' buckets and the file server stay as they are.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala:
##########
@@ -3251,8 +3251,9 @@ class HiveDDLSuite
val udtfJar = TestUDTFJar.jar
val jarName = udtfJar.getName
val jar = udtfJar.toURI.toString
+ // listJars() flattens every artifact bucket, so clear the jar from all
of them.
spark.sparkContext.allAddedJars.keys.find(_.contains(jarName))
Review Comment:
Good catch. This went away with the cleanup: a uniquely named jar has no
stale keys under any scheme.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala:
##########
@@ -3251,8 +3251,9 @@ class HiveDDLSuite
val udtfJar = TestUDTFJar.jar
val jarName = udtfJar.getName
val jar = udtfJar.toURI.toString
+ // listJars() flattens every artifact bucket, so clear the jar from all
of them.
Review Comment:
I tried this locally, and `ADD JAR` alone does not reach a non-default
bucket in this suite. `TestHive` sets
`spark.sql.artifact.isolation.enabled=false` (the SPARK-50244 TODO in
`TestHive.overrideConfs`). A probe printed `isolation=false` and the jar under
the `"default"` bucket, and with that `ADD JAR` added, the current master
version of the test also passes. So my PR description overstated how a
non-default bucket happens here; I have corrected it. With the unique jar, the
test no longer depends on which bucket anything lands in, so I did not add the
`ADD JAR` step.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala:
##########
@@ -3251,8 +3251,9 @@ class HiveDDLSuite
val udtfJar = TestUDTFJar.jar
Review Comment:
Good idea, done. The test now copies `TestUDTFJar.jar` to a temp file with a
unique name, so no earlier test can have registered it, and the cleanup is
gone. This also removes the dependency on how `addedJars` is keyed.
##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala:
##########
@@ -3251,8 +3251,9 @@ class HiveDDLSuite
val udtfJar = TestUDTFJar.jar
val jarName = udtfJar.getName
val jar = udtfJar.toURI.toString
+ // listJars() flattens every artifact bucket, so clear the jar from all
of them.
spark.sparkContext.allAddedJars.keys.find(_.contains(jarName))
Review Comment:
Removed along with the cleanup.
--
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]
