xkrogen commented on a change in pull request #33101:
URL: https://github.com/apache/spark/pull/33101#discussion_r663133500



##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -285,9 +285,10 @@ private[spark] object Utils extends Logging {
    */
   def createDirectory(dir: File): Boolean = {
     try {
-      // This sporadically fails - not sure why ... !dir.exists() && 
!dir.mkdirs()
-      // So attempting to create and then check if directory was created or 
not.
-      dir.mkdirs()
+      // SPARK-35907
+      // This could throw more meaningful exception information if directory 
creation failed.

Review comment:
       I kind of feel the comments aren't necessary? The comment explains why 
we switched from `File#mkdirs()` to `Files.createDirectories()`, but IMO a 
comment should explain something that is confusing or unexpected, and if you 
just come across `Files.createDirectories()`, there's nothing strange about 
that.




-- 
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: reviews-unsubscr...@spark.apache.org

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

Reply via email to