[GitHub] [hudi] manojpec commented on a change in pull request #3977: [HUDI-2716] InLineFS path conversion support for S3 FS paths

[GitBox <gi...@apache.org>]

manojpec commented on a change in pull request #3977:
URL: https://github.com/apache/hudi/pull/3977#discussion_r747936234



##########
File path: hudi-common/src/main/java/org/apache/hudi/common/fs/inline/InLineFSUtils.java
##########
@@ -54,21 +57,26 @@ public static Path getInlineFilePath(Path outerPath, String origScheme, long inL
   }
 
   /**
-   * Inline file format
-   * "inlinefs://<path_to_outer_file>/<outer_file_scheme>/?start_offset=start_offset>&length=<length>"
-   * Outer File format
-   * "<outer_file_scheme>://<path_to_outer_file>"
+   * InlineFS Path format:
+   * "inlinefs://path/to/outer/file/outer_file_schema/?start_offset=start_offset>&length=<length>"
    * <p>
-   * Eg input : "inlinefs://file1/sa3/?start_offset=20&length=40".
-   * Output : "sa3://file1"
+   * Outer File Path format:
+   * "outer_file_schema://path/to/outer/file"
+   * <p>
+   * Example
+   * Input: "inlinefs://file1/s3a/?start_offset=20&length=40".

Review comment:
       I have a different thought on this. The existing inlinefs paths doesn't follow the right URI format. URIs have the format `<schema>://path/to/file/?options=args&..`. So, instead of encoding  number of slashes, we need to make all encoded/converted paths to comply to standard format which includes the schema separator `://`. But doing so will change a quite a few of existing inlinefs path handling and maybe not the right idea to do late in the release. I can file a new ticket to take it up for .11 to make inlinefs paths better. your thoughts please?




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org