[#571][FOLLOWUP] fix: optimize base dir init process #616
Conversation
Codecov Report
@@ Coverage Diff @@
## master #616 +/- ##
============================================
+ Coverage 60.87% 60.90% +0.03%
+ Complexity 1802 1799 -3
============================================
Files 214 214
Lines 12388 12381 -7
Branches 1045 1042 -3
============================================
Hits 7541 7541
+ Misses 4442 4437 -5
+ Partials 405 403 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
LGTM, thanks @advancedxy. Can you add a test to cover your change?
Let me check that. This behavior should already been covered by existing UTs. |
To my surprise, the else branch is not covered.
|
storage/src/main/java/org/apache/uniffle/storage/common/LocalStorage.java
Outdated
Show resolved
Hide resolved
storage/src/main/java/org/apache/uniffle/storage/common/LocalStorage.java
Outdated
Show resolved
Hide resolved
kaijchen
changed the title
|
Should this pr be included in version 0.7? |
Yes. |
If this pr is merged, we should backport this pr to 0.7. |
advancedxy
force-pushed
the
issue_571
branch
from
0194e0a
to
a5e4bd9
Compare
storage/src/main/java/org/apache/uniffle/storage/common/LocalStorage.java
Outdated
Show resolved
Hide resolved
advancedxy
force-pushed
the
issue_571
branch
from
a5e4bd9
to
8dea72b
Compare
|
cc @zuston |
It's OK for me. |
There was a problem hiding this comment.
Thanks @advancedxy, this is a lot better than previous. Just some minor comments.
storage/src/main/java/org/apache/uniffle/storage/common/LocalStorage.java
Outdated
Show resolved
Hide resolved
advancedxy
force-pushed
the
issue_571
branch
from
8dea72b
to
6407a75
Compare
advancedxy
changed the title
advancedxy
force-pushed
the
issue_571
branch
from
6407a75
to
3bb35d3
Compare
thanks @zuston, I will raise PR to branch-0.7, once this is merged. |
…he#616) ### What changes were proposed in this pull request? optimize the base dir init logic, it now skips recreate base dir if it's already an dir ### Why are the changes needed? handles some corner cases such as the base dir is an mount point root path. ### Does this PR introduce _any_ user-facing change? rss shuffle server can use mounted path as base dir directly. ### How was this patch tested? Existing UTs.
#622) ### What changes were proposed in this pull request? optimize the base dir init logic, it now skips recreate base dir if it's already an dir ### Why are the changes needed? handles some corner cases such as the base dir is an mount point root path. ### Does this PR introduce _any_ user-facing change? rss shuffle server can use mounted path as base dir directly. ### How was this patch tested? Existing UTs.
What changes were proposed in this pull request?
optimize the base dir init logic, it now skips recreate base dir if it's already an dir
Why are the changes needed?
handles some corner cases such as the base dir is an mount point root path.
Does this PR introduce any user-facing change?
rss shuffle server can use mounted path as base dir directly.
How was this patch tested?
Existing UTs.