You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@submarine.apache.org by li...@apache.org on 2020/03/10 03:27:11 UTC

[submarine] branch master updated: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent

This is an automated email from the ASF dual-hosted git repository.

liuxun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/submarine.git


The following commit(s) were added to refs/heads/master by this push:
     new 687ca62  SUBMARINE-78. Make RemoteDirectoryManager interface more consistent
687ca62 is described below

commit 687ca6203173a7b7dc92b2bc4427c6b87a3dfec3
Author: cchung100m <cc...@cs.ccu.edu.tw>
AuthorDate: Tue Mar 10 08:02:19 2020 +0800

    SUBMARINE-78. Make RemoteDirectoryManager interface more consistent
    
    ### What is this PR for?
    
    RemoteDirectoryManager contains many methods that receive a URI.
    The types are sometimes Strings, sometimes Path.
    We need to make this more consistent.
    
    ### What type of PR is it?
    [Improvement | Refactoring]
    
    ### What is the Jira issue?
    
    https://issues.apache.org/jira/browse/SUBMARINE-78
    
    ### How should this be tested?
    
    https://travis-ci.com/cchung100m/submarine/builds/152425410
    
    ### Screenshots (if appropriate)
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No
    
    Author: cchung100m <cc...@cs.ccu.edu.tw>
    
    Closes #214 from cchung100m/SUBMARINE-78 and squashes the following commits:
    
    cafcb12 [cchung100m] Align parameter name
    998d257 [cchung100m] Make RemoteDirectoryManager interface more consistent
---
 .../commons/runtime/fs/DefaultRemoteDirectoryManager.java      |  8 ++++----
 .../submarine/commons/runtime/fs/RemoteDirectoryManager.java   |  4 ++--
 .../commons/runtime/fs/MockRemoteDirectoryManager.java         |  9 +++++----
 .../server/submitter/yarnservice/FileSystemOperations.java     |  2 +-
 .../server/submitter/yarnservice/utils/Localizer.java          |  3 +--
 .../server/submitter/yarnservice/utils/LocalizerTest.java      | 10 +++++-----
 6 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/submarine-commons/commons-runtime/src/main/java/org/apache/submarine/commons/runtime/fs/DefaultRemoteDirectoryManager.java b/submarine-commons/commons-runtime/src/main/java/org/apache/submarine/commons/runtime/fs/DefaultRemoteDirectoryManager.java
index 52e9fab..c7de978 100644
--- a/submarine-commons/commons-runtime/src/main/java/org/apache/submarine/commons/runtime/fs/DefaultRemoteDirectoryManager.java
+++ b/submarine-commons/commons-runtime/src/main/java/org/apache/submarine/commons/runtime/fs/DefaultRemoteDirectoryManager.java
@@ -134,13 +134,13 @@ public class DefaultRemoteDirectoryManager implements RemoteDirectoryManager {
   }
 
   @Override
-  public boolean existsRemoteFile(Path url) throws IOException {
-    return getFileSystemByUri(url.toUri().toString()).exists(url);
+  public boolean existsRemoteFile(String uri) throws IOException {
+    return getFileSystemByUri(uri).exists(new Path(uri));
   }
 
   @Override
-  public FileStatus getRemoteFileStatus(Path url) throws IOException {
-    return getFileSystemByUri(url.toUri().toString()).getFileStatus(url);
+  public FileStatus getRemoteFileStatus(String uri) throws IOException {
+    return getFileSystemByUri(uri).getFileStatus(new Path(uri));
   }
 
   @Override
diff --git a/submarine-commons/commons-runtime/src/main/java/org/apache/submarine/commons/runtime/fs/RemoteDirectoryManager.java b/submarine-commons/commons-runtime/src/main/java/org/apache/submarine/commons/runtime/fs/RemoteDirectoryManager.java
index b5baa81..bac953a 100644
--- a/submarine-commons/commons-runtime/src/main/java/org/apache/submarine/commons/runtime/fs/RemoteDirectoryManager.java
+++ b/submarine-commons/commons-runtime/src/main/java/org/apache/submarine/commons/runtime/fs/RemoteDirectoryManager.java
@@ -45,9 +45,9 @@ public interface RemoteDirectoryManager {
   boolean copyRemoteToLocal(String remoteUri, String localUri)
       throws IOException;
 
-  boolean existsRemoteFile(Path uri) throws IOException;
+  boolean existsRemoteFile(String uri) throws IOException;
 
-  FileStatus getRemoteFileStatus(Path uri) throws IOException;
+  FileStatus getRemoteFileStatus(String uri) throws IOException;
 
   long getRemoteFileSize(String uri) throws IOException;
 }
diff --git a/submarine-commons/commons-runtime/src/test/java/org/apache/submarine/commons/runtime/fs/MockRemoteDirectoryManager.java b/submarine-commons/commons-runtime/src/test/java/org/apache/submarine/commons/runtime/fs/MockRemoteDirectoryManager.java
index f3ea39b..abc0bf7 100644
--- a/submarine-commons/commons-runtime/src/test/java/org/apache/submarine/commons/runtime/fs/MockRemoteDirectoryManager.java
+++ b/submarine-commons/commons-runtime/src/test/java/org/apache/submarine/commons/runtime/fs/MockRemoteDirectoryManager.java
@@ -183,16 +183,17 @@ public class MockRemoteDirectoryManager implements RemoteDirectoryManager {
   }
 
   @Override
-  public boolean existsRemoteFile(Path uri) throws IOException {
+  public boolean existsRemoteFile(String uri) throws IOException {
+    String dirName = new Path(uri).getName();
     String fakeLocalFilePath = this.jobDir.getAbsolutePath()
-        + "/" + uri.getName();
+        + "/" + dirName;
     return new File(fakeLocalFilePath).exists();
   }
 
   @Override
-  public FileStatus getRemoteFileStatus(Path p) throws IOException {
+  public FileStatus getRemoteFileStatus(String uri) throws IOException {
     return getDefaultFileSystem().getFileStatus(new Path(
-        convertToStagingPath(p.toUri().toString())));
+        convertToStagingPath(uri)));
   }
 
   @Override
diff --git a/submarine-server/server-submitter/submitter-yarnservice/src/main/java/org/apache/submarine/server/submitter/yarnservice/FileSystemOperations.java b/submarine-server/server-submitter/submitter-yarnservice/src/main/java/org/apache/submarine/server/submitter/yarnservice/FileSystemOperations.java
index 197f0cc..f08c5ae 100644
--- a/submarine-server/server-submitter/submitter-yarnservice/src/main/java/org/apache/submarine/server/submitter/yarnservice/FileSystemOperations.java
+++ b/submarine-server/server-submitter/submitter-yarnservice/src/main/java/org/apache/submarine/server/submitter/yarnservice/FileSystemOperations.java
@@ -119,7 +119,7 @@ public class FileSystemOperations {
     if (remote) {
       // Append original modification time and size to zip file name
       FileStatus status =
-          remoteDirectoryManager.getRemoteFileStatus(new Path(remoteDir));
+          remoteDirectoryManager.getRemoteFileStatus(remoteDir);
       suffix = getSuffixOfRemoteDirectory(remoteDir, status);
       // Download them to temp dir
       downloadRemoteFile(remoteDir, destFilePath);
diff --git a/submarine-server/server-submitter/submitter-yarnservice/src/main/java/org/apache/submarine/server/submitter/yarnservice/utils/Localizer.java b/submarine-server/server-submitter/submitter-yarnservice/src/main/java/org/apache/submarine/server/submitter/yarnservice/utils/Localizer.java
index eac91a1..8e59135 100644
--- a/submarine-server/server-submitter/submitter-yarnservice/src/main/java/org/apache/submarine/server/submitter/yarnservice/utils/Localizer.java
+++ b/submarine-server/server-submitter/submitter-yarnservice/src/main/java/org/apache/submarine/server/submitter/yarnservice/utils/Localizer.java
@@ -170,10 +170,9 @@ public class Localizer {
     String remoteUri;
     for (Localization localization : localizations) {
       remoteUri = localization.getRemoteUri();
-      Path resourceToLocalize = new Path(remoteUri);
 
       if (remoteDirectoryManager.isRemote(remoteUri)) {
-        if (!remoteDirectoryManager.existsRemoteFile(resourceToLocalize)) {
+        if (!remoteDirectoryManager.existsRemoteFile(remoteUri)) {
           throw new FileNotFoundException(
               "File " + remoteUri + " doesn't exists.");
         }
diff --git a/submarine-server/server-submitter/submitter-yarnservice/src/test/java/org/apache/submarine/server/submitter/yarnservice/utils/LocalizerTest.java b/submarine-server/server-submitter/submitter-yarnservice/src/test/java/org/apache/submarine/server/submitter/yarnservice/utils/LocalizerTest.java
index 0a7886b..53e2e9d 100644
--- a/submarine-server/server-submitter/submitter-yarnservice/src/test/java/org/apache/submarine/server/submitter/yarnservice/utils/LocalizerTest.java
+++ b/submarine-server/server-submitter/submitter-yarnservice/src/test/java/org/apache/submarine/server/submitter/yarnservice/utils/LocalizerTest.java
@@ -98,7 +98,7 @@ public class LocalizerTest {
 
   private void testLocalizeExistingRemoteFileInternal() throws IOException {
     when(remoteDirectoryManager.isRemote(anyString())).thenReturn(true);
-    when(remoteDirectoryManager.existsRemoteFile(any(Path.class)))
+    when(remoteDirectoryManager.existsRemoteFile(anyString()))
         .thenReturn(true);
 
     Localization localization = new Localization();
@@ -138,7 +138,7 @@ public class LocalizerTest {
   @Test(expected = FileNotFoundException.class)
   public void testLocalizeNotExistingRemoteFile() throws IOException {
     when(remoteDirectoryManager.isRemote(anyString())).thenReturn(true);
-    when(remoteDirectoryManager.existsRemoteFile(any(Path.class)))
+    when(remoteDirectoryManager.existsRemoteFile(anyString()))
         .thenReturn(false);
 
     Localization localization = new Localization();
@@ -244,7 +244,7 @@ public class LocalizerTest {
       throws IOException {
     when(remoteDirectoryManager.isDir(anyString())).thenReturn(true);
     when(remoteDirectoryManager.isRemote(anyString())).thenReturn(true);
-    when(remoteDirectoryManager.existsRemoteFile(any(Path.class)))
+    when(remoteDirectoryManager.existsRemoteFile(anyString()))
         .thenReturn(true);
     String remoteUri = "hdfs://remotedir1/remotedir2";
     when(fsOperations.uploadToRemoteFile(any(Path.class), anyString()))
@@ -265,7 +265,7 @@ public class LocalizerTest {
   public void testLocalizeExistingRemoteDirectory() throws IOException {
     when(remoteDirectoryManager.isDir(anyString())).thenReturn(true);
     when(remoteDirectoryManager.isRemote(anyString())).thenReturn(true);
-    when(remoteDirectoryManager.existsRemoteFile(any(Path.class)))
+    when(remoteDirectoryManager.existsRemoteFile(anyString()))
         .thenReturn(true);
     String remoteUri = "hdfs://remotedir1/remotedir2";
     when(fsOperations.uploadToRemoteFile(any(Path.class), anyString()))
@@ -303,7 +303,7 @@ public class LocalizerTest {
   @Test
   public void testLocalizeNonHdfsRemoteUri() throws IOException {
     when(remoteDirectoryManager.isRemote(anyString())).thenReturn(true);
-    when(remoteDirectoryManager.existsRemoteFile(any(Path.class)))
+    when(remoteDirectoryManager.existsRemoteFile(anyString()))
         .thenReturn(true);
     String remoteUri = "remote://remotedir1/remotedir2";
     when(fsOperations.uploadToRemoteFile(any(Path.class), anyString()))


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@submarine.apache.org
For additional commands, e-mail: dev-help@submarine.apache.org