Shockang commented on a change in pull request #33101:
URL: https://github.com/apache/spark/pull/33101#discussion_r662707815
##########
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.
+ // To be on the safe side, try to create and then check if directory was
created or not.
+ Files.createDirectories(dir.toPath)
if ( !dir.exists() || !dir.isDirectory) {
logError(s"Failed to create directory " + dir)
}
Review comment:
> The word "contract" is not used, but I think that is rarely the case
in method descriptions. For example none of the methods on `Dataset` explicitly
state contracts or guarantees, but we are expected to assume that the method
will do what it says it does, and indicate an error condition using an
exception otherwise. My impression here is that the weird semantics of
`File#mkdirs()` are translating into FUD about the potential shortcomings of
`Files.createDirectories()` and that, if the method were taken without that
historical context, we wouldn't be worrying about it.
>
> This all being said, it's only three lines of code extra, so of course it
is no big detriment to be on the safe side.
I agree with you in terms of safety and minimizing modification points.Do
you have any questions with my current code?
--
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]
