This is an automated email from the ASF dual-hosted git repository.
xianjingfeng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/uniffle.git
The following commit(s) were added to refs/heads/master by this push:
new 890758bb9 fix(test): Fix test about default storage type for
DefaultStorageMediaProviderTest (#2419)
890758bb9 is described below
commit 890758bb9883d3c7a9375e6c64a6f3662fd96cfc
Author: Andras Salamon <[email protected]>
AuthorDate: Thu Apr 10 11:44:02 2025 +0200
fix(test): Fix test about default storage type for
DefaultStorageMediaProviderTest (#2419)
### What changes were proposed in this pull request?
This part of testStorageProvider should test the default storage media for
a non-existent directory. But 47cac39 changed the behavior of
getStorageMediaFor, now it checks the parent directory (recursively) so for
/path/to/base/dir it will end up checking / which is existent. With this change
it will check path which is missing, so it will really check the default value.
### Why are the changes needed?
### Does this PR introduce any user-facing change?
No.
### How was this patch tested?
UT
---
.../uniffle/storage/common/DefaultStorageMediaProvider.java | 13 +++++++++++--
.../storage/common/DefaultStorageMediaProviderTest.java | 5 +++--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git
a/storage/src/main/java/org/apache/uniffle/storage/common/DefaultStorageMediaProvider.java
b/storage/src/main/java/org/apache/uniffle/storage/common/DefaultStorageMediaProvider.java
index b2ad3f706..622aad903 100644
---
a/storage/src/main/java/org/apache/uniffle/storage/common/DefaultStorageMediaProvider.java
+++
b/storage/src/main/java/org/apache/uniffle/storage/common/DefaultStorageMediaProvider.java
@@ -24,6 +24,7 @@ import java.net.URISyntaxException;
import java.nio.file.FileStore;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.Arrays;
import java.util.List;
@@ -40,6 +41,7 @@ public class DefaultStorageMediaProvider implements
StorageMediaProvider {
private static final String NUMBERIC_STRING = "0123456789";
private static final String BLOCK_PATH_FORMAT =
"/sys/block/%s/queue/rotational";
private static final String HDFS = "hdfs";
+ private static final String FILE = "file";
private static final List<String> OBJECT_STORE_SCHEMAS =
Arrays.asList("s3", "oss", "cos", "gcs", "obs", "daos");
@@ -64,6 +66,13 @@ public class DefaultStorageMediaProvider implements
StorageMediaProvider {
try {
File baseFile = new File(baseDir);
FileStore store = getFileStore(baseFile.toPath());
+ if (store == null) {
+ URI uri = new URI(baseDir);
+ String scheme = uri.getScheme();
+ if (FILE.equals(scheme)) {
+ store = getFileStore(Paths.get(uri));
+ }
+ }
if (store == null) {
throw new IOException("Can't get FileStore for path:" +
baseFile.getAbsolutePath());
}
@@ -81,8 +90,8 @@ public class DefaultStorageMediaProvider implements
StorageMediaProvider {
}
}
}
- } catch (IOException ioe) {
- logger.warn("Get storage type failed with exception", ioe);
+ } catch (IOException | URISyntaxException e) {
+ logger.warn("Get storage type failed with exception", e);
}
}
logger.info("Default storage type provider returns HDD by default");
diff --git
a/storage/src/test/java/org/apache/uniffle/storage/common/DefaultStorageMediaProviderTest.java
b/storage/src/test/java/org/apache/uniffle/storage/common/DefaultStorageMediaProviderTest.java
index 82fe98359..9ded908a1 100644
---
a/storage/src/test/java/org/apache/uniffle/storage/common/DefaultStorageMediaProviderTest.java
+++
b/storage/src/test/java/org/apache/uniffle/storage/common/DefaultStorageMediaProviderTest.java
@@ -40,8 +40,9 @@ public class DefaultStorageMediaProviderTest {
StorageMedia.OBJECT_STORE,
provider.getStorageMediaFor("cos://bucket-name/b/path"));
// by default, the local file should report as HDD
- assertEquals(StorageMedia.HDD,
provider.getStorageMediaFor("/path/to/base/dir"));
- assertEquals(StorageMedia.HDD,
provider.getStorageMediaFor("file:///path/to/a/dir"));
+ assertEquals(StorageMedia.HDD,
provider.getStorageMediaFor("path/to/base/dir"));
+ assertEquals(
+ provider.getStorageMediaFor("/"),
provider.getStorageMediaFor("file:///path/to/a/dir"));
// invalid uri should also be reported as HDD
assertEquals(StorageMedia.HDD,
provider.getStorageMediaFor("file@xx:///path/to/a"));