[SpotBugs] Fix REC_CATCH_EXCEPTION #527
Conversation
Codecov Report
@@ Coverage Diff @@
## master #527 +/- ##
============================================
+ Coverage 60.11% 60.21% +0.10%
- Complexity 1769 1784 +15
============================================
Files 205 205
Lines 11532 11557 +25
Branches 1033 1042 +9
============================================
+ Hits 6932 6959 +27
+ Misses 4194 4190 -4
- Partials 406 408 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Cloud you add an umbrella issue to track all the spotbugs improvements? |
storage/src/main/java/org/apache/uniffle/storage/handler/impl/LocalFileWriteHandler.java
Outdated
Show resolved
Hide resolved
client-mr/src/main/java/org/apache/hadoop/mapreduce/v2/app/RssMRAppMaster.java
Outdated
Show resolved
Hide resolved
client-spark/spark3/src/main/java/org/apache/spark/shuffle/RssShuffleManager.java
Outdated
Show resolved
Hide resolved
| if (!baseFolder.exists()) { | ||
| try { | ||
| // try to create folder, it may be created by other Shuffle Server | ||
| baseFolder.mkdirs(); |
There was a problem hiding this comment.
Return value is ignored: false means failure.
storage/src/main/java/org/apache/uniffle/storage/handler/impl/LocalFileWriteHandler.java
Show resolved
Hide resolved
| if (!baseFolder.isDirectory() && !baseFolder.mkdirs()) { | ||
| // if folder is not created successfully, make sure it exists | ||
| if (!baseFolder.isDirectory()) { | ||
| LOG.error("Can't create shuffle folder: {}", basePath); |
There was a problem hiding this comment.
the logging should be more verbose about whether this is a file or not.
Another things is that when basePath could not be created, a runtime exception should be raised.
There was a problem hiding this comment.
After a second thought, let's just focus on spotbug issues in this PR.
The createBasePath() can be fixed in another PR.
|
Thanks @advancedxy for the review. |
What changes were proposed in this pull request?
Why are the changes needed?
Sub-task of #532 - Fix REC_CATCH_EXCEPTION.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
CI.