Skip to content

Commit 890758b

Browse files
authored
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
1 parent 9007bed commit 890758b

2 files changed

Lines changed: 14 additions & 4 deletions

File tree

storage/src/main/java/org/apache/uniffle/storage/common/DefaultStorageMediaProvider.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.nio.file.FileStore;
2525
import java.nio.file.Files;
2626
import java.nio.file.Path;
27+
import java.nio.file.Paths;
2728
import java.util.Arrays;
2829
import java.util.List;
2930

@@ -40,6 +41,7 @@ public class DefaultStorageMediaProvider implements StorageMediaProvider {
4041
private static final String NUMBERIC_STRING = "0123456789";
4142
private static final String BLOCK_PATH_FORMAT = "/sys/block/%s/queue/rotational";
4243
private static final String HDFS = "hdfs";
44+
private static final String FILE = "file";
4345
private static final List<String> OBJECT_STORE_SCHEMAS =
4446
Arrays.asList("s3", "oss", "cos", "gcs", "obs", "daos");
4547

@@ -64,6 +66,13 @@ public StorageMedia getStorageMediaFor(String baseDir) {
6466
try {
6567
File baseFile = new File(baseDir);
6668
FileStore store = getFileStore(baseFile.toPath());
69+
if (store == null) {
70+
URI uri = new URI(baseDir);
71+
String scheme = uri.getScheme();
72+
if (FILE.equals(scheme)) {
73+
store = getFileStore(Paths.get(uri));
74+
}
75+
}
6776
if (store == null) {
6877
throw new IOException("Can't get FileStore for path:" + baseFile.getAbsolutePath());
6978
}
@@ -81,8 +90,8 @@ public StorageMedia getStorageMediaFor(String baseDir) {
8190
}
8291
}
8392
}
84-
} catch (IOException ioe) {
85-
logger.warn("Get storage type failed with exception", ioe);
93+
} catch (IOException | URISyntaxException e) {
94+
logger.warn("Get storage type failed with exception", e);
8695
}
8796
}
8897
logger.info("Default storage type provider returns HDD by default");

storage/src/test/java/org/apache/uniffle/storage/common/DefaultStorageMediaProviderTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ public void testStorageProvider() {
4040
StorageMedia.OBJECT_STORE, provider.getStorageMediaFor("cos://bucket-name/b/path"));
4141

4242
// by default, the local file should report as HDD
43-
assertEquals(StorageMedia.HDD, provider.getStorageMediaFor("/path/to/base/dir"));
44-
assertEquals(StorageMedia.HDD, provider.getStorageMediaFor("file:///path/to/a/dir"));
43+
assertEquals(StorageMedia.HDD, provider.getStorageMediaFor("path/to/base/dir"));
44+
assertEquals(
45+
provider.getStorageMediaFor("/"), provider.getStorageMediaFor("file:///path/to/a/dir"));
4546

4647
// invalid uri should also be reported as HDD
4748
assertEquals(StorageMedia.HDD, provider.getStorageMediaFor("file@xx:///path/to/a"));

0 commit comments

Comments
 (0)