You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@submarine.apache.org by GitBox <gi...@apache.org> on 2020/03/09 13:21:54 UTC

[GitHub] [submarine] cchung100m opened a new pull request #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent

cchung100m opened a new pull request #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent
URL: https://github.com/apache/submarine/pull/214
 
 
   ### 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
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [submarine] cchung100m commented on a change in pull request #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent

Posted by GitBox <gi...@apache.org>.
cchung100m commented on a change in pull request #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent
URL: https://github.com/apache/submarine/pull/214#discussion_r390042276
 
 

 ##########
 File path: submarine-commons/commons-runtime/src/main/java/org/apache/submarine/commons/runtime/fs/DefaultRemoteDirectoryManager.java
 ##########
 @@ -134,13 +134,13 @@ public boolean copyRemoteToLocal(String remoteUri, String localUri)
   }
 
   @Override
-  public boolean existsRemoteFile(Path url) throws IOException {
-    return getFileSystemByUri(url.toUri().toString()).exists(url);
+  public boolean existsRemoteFile(String remoteUri) throws IOException {
+    return getFileSystemByUri(remoteUri).exists(new Path(remoteUri));
   }
 
   @Override
-  public FileStatus getRemoteFileStatus(Path url) throws IOException {
-    return getFileSystemByUri(url.toUri().toString()).getFileStatus(url);
+  public FileStatus getRemoteFileStatus(String remoteUri) throws IOException {
 
 Review comment:
   Hi @yuanzac 
   
   Thanks for the review. I updated the part you mentioned.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [submarine] liuxunorg commented on issue #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent

Posted by GitBox <gi...@apache.org>.
liuxunorg commented on issue #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent
URL: https://github.com/apache/submarine/pull/214#issuecomment-596576690
 
 
   @cchung100m This PR travis not passed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [submarine] cchung100m commented on issue #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent

Posted by GitBox <gi...@apache.org>.
cchung100m commented on issue #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent
URL: https://github.com/apache/submarine/pull/214#issuecomment-596856574
 
 
   Hi @liuxunorg 
   
   Thanks for the feedback. I re-triggered the CI process.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [submarine] cchung100m commented on issue #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent

Posted by GitBox <gi...@apache.org>.
cchung100m commented on issue #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent
URL: https://github.com/apache/submarine/pull/214#issuecomment-596884111
 
 
   Thanks. @liuxunorg 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [submarine] cchung100m commented on issue #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent

Posted by GitBox <gi...@apache.org>.
cchung100m commented on issue #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent
URL: https://github.com/apache/submarine/pull/214#issuecomment-596567532
 
 
   Hi @szilard-nemeth 
   
   I would appreciate that if you can help to review it, thanks.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [submarine] yuanzac commented on a change in pull request #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent

Posted by GitBox <gi...@apache.org>.
yuanzac commented on a change in pull request #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent
URL: https://github.com/apache/submarine/pull/214#discussion_r389785192
 
 

 ##########
 File path: submarine-commons/commons-runtime/src/main/java/org/apache/submarine/commons/runtime/fs/DefaultRemoteDirectoryManager.java
 ##########
 @@ -134,13 +134,13 @@ public boolean copyRemoteToLocal(String remoteUri, String localUri)
   }
 
   @Override
-  public boolean existsRemoteFile(Path url) throws IOException {
-    return getFileSystemByUri(url.toUri().toString()).exists(url);
+  public boolean existsRemoteFile(String remoteUri) throws IOException {
+    return getFileSystemByUri(remoteUri).exists(new Path(remoteUri));
   }
 
   @Override
-  public FileStatus getRemoteFileStatus(Path url) throws IOException {
-    return getFileSystemByUri(url.toUri().toString()).getFileStatus(url);
+  public FileStatus getRemoteFileStatus(String remoteUri) throws IOException {
 
 Review comment:
   We'd better use change getRemoteFileStatus(String remoteUri) to getRemoteFileStatus(String uri). So that the parameter name uri is the same as the one of the interface RemoteDirectoryManager

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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

[GitHub] [submarine] asfgit closed pull request #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #214: SUBMARINE-78. Make RemoteDirectoryManager interface more consistent
URL: https://github.com/apache/submarine/pull/214
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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