Skip to content

Conversation

@viirya
Copy link
Member

@viirya viirya commented Nov 25, 2014

File.exists() and File.mkdirs() only throw SecurityException instead of IOException. Then, when an exception is thrown, dir should be reset too.

@JoshRosen
Copy link
Contributor

Jenkins, this is ok to test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like these two methods can't throw IOException after all, is that the gist of it? mkdirs just returns false if it fails, hm. https://docs.oracle.com/javase/7/docs/api/java/io/File.html#mkdirs()

dir = null is a good bug fix. I might have changed this to not even assign dir and hold the new File in a temp variable until the checks succeeded. This looks equivalent though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The only exception they would throw is SecurityException. And when mkdirs finds failures other than SecurityException it just returns false.

@SparkQA
Copy link

SparkQA commented Nov 25, 2014

Test build #23831 has finished for PR 3449 at commit 36cacbd.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@viirya
Copy link
Member Author

viirya commented Nov 27, 2014

@JoshRosen @srowen Any other comments? Is this ok to be merged?

@JoshRosen
Copy link
Contributor

LGTM, so I'm going to merge this into master and the maintenance branches. Thanks!

asfgit pushed a commit that referenced this pull request Nov 29, 2014
…empDir()

`File.exists()` and `File.mkdirs()` only throw `SecurityException` instead of `IOException`. Then, when an exception is thrown, `dir` should be reset too.

Author: Liang-Chi Hsieh <[email protected]>

Closes #3449 from viirya/fix_createtempdir and squashes the following commits:

36cacbd [Liang-Chi Hsieh] Use proper exception and reset variable.

(cherry picked from commit 49fe879)
Signed-off-by: Josh Rosen <[email protected]>
@asfgit asfgit closed this in 49fe879 Nov 29, 2014
asfgit pushed a commit that referenced this pull request Nov 29, 2014
…empDir()

`File.exists()` and `File.mkdirs()` only throw `SecurityException` instead of `IOException`. Then, when an exception is thrown, `dir` should be reset too.

Author: Liang-Chi Hsieh <[email protected]>

Closes #3449 from viirya/fix_createtempdir and squashes the following commits:

36cacbd [Liang-Chi Hsieh] Use proper exception and reset variable.

(cherry picked from commit 49fe879)
Signed-off-by: Josh Rosen <[email protected]>
asfgit pushed a commit that referenced this pull request Nov 29, 2014
…empDir()

`File.exists()` and `File.mkdirs()` only throw `SecurityException` instead of `IOException`. Then, when an exception is thrown, `dir` should be reset too.

Author: Liang-Chi Hsieh <[email protected]>

Closes #3449 from viirya/fix_createtempdir and squashes the following commits:

36cacbd [Liang-Chi Hsieh] Use proper exception and reset variable.

(cherry picked from commit 49fe879)
Signed-off-by: Josh Rosen <[email protected]>
@viirya viirya deleted the fix_createtempdir branch December 27, 2023 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants