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"));

Reply via email to