MartijnVisser commented on code in PR #20011:
URL: https://github.com/apache/flink/pull/20011#discussion_r901309667


##########
flink-core/src/main/java/org/apache/flink/core/fs/FileSystem.java:
##########
@@ -527,13 +527,30 @@ public static FileSystem getUnguardedFileSystem(final URI 
fsUri) throws IOExcept
                 try {
                     fs = FALLBACK_FACTORY.create(uri);
                 } catch (UnsupportedFileSystemSchemeException e) {
-                    throw new UnsupportedFileSystemSchemeException(
-                            "Could not find a file system implementation for 
scheme '"
-                                    + uri.getScheme()
-                                    + "'. The scheme is not directly supported 
by Flink and no Hadoop file system to "
-                                    + "support this scheme could be loaded. 
For a full list of supported file systems, "
-                                    + "please see 
https://nightlies.apache.org/flink/flink-docs-stable/ops/filesystems/.";,
-                            e);
+                    if 
(DIRECTLY_SUPPORTED_FILESYSTEM.containsKey(uri.getScheme())) {
+                        final Collection<String> plugins =
+                                
DIRECTLY_SUPPORTED_FILESYSTEM.get(uri.getScheme());
+                        throw new UnsupportedFileSystemSchemeException(
+                                String.format(
+                                        "Could not find a file system 
implementation for scheme '%s'. The scheme is "
+                                                + "directly supported by Flink 
through the following plugin%s: %s "

Review Comment:
   I still feel like this is kind of a grammatical discussion about "directly 
supported" (which I also saw happening on the ticket). 
   
   You can have two explanations for _directly_:
   * I don't have to do anything as a user since Flink supports it out of the 
box
   * I do have to do something since Flink supports it but I have to 
configure/do something
   
   I think the intent was to let the user know that the second situation was 
applicable.
   
   With this change, I don't think it makes sense to say "the scheme is 
directly supported by Flink". I think we want to inform the user that something 
is wrong and action needs to be taken by the user. So I would phrase it like
   
   ```
   Could not find a file system implementation for scheme '%s'. File system 
schemes are supported by Flink through the following plugin(s): 
   
   No file system to support this scheme could be loaded. Please ensure that 
each plugin is configured properly and resides within its own subfolder in the 
plugins directory. See 
https://nightlies.apache.org/flink/flink-docs-stable/docs/deployment/filesystems/plugins/
 for more information
   ```



##########
flink-core/src/main/java/org/apache/flink/core/fs/FileSystem.java:
##########
@@ -527,13 +527,30 @@ public static FileSystem getUnguardedFileSystem(final URI 
fsUri) throws IOExcept
                 try {
                     fs = FALLBACK_FACTORY.create(uri);
                 } catch (UnsupportedFileSystemSchemeException e) {
-                    throw new UnsupportedFileSystemSchemeException(
-                            "Could not find a file system implementation for 
scheme '"
-                                    + uri.getScheme()
-                                    + "'. The scheme is not directly supported 
by Flink and no Hadoop file system to "
-                                    + "support this scheme could be loaded. 
For a full list of supported file systems, "
-                                    + "please see 
https://nightlies.apache.org/flink/flink-docs-stable/ops/filesystems/.";,
-                            e);
+                    if 
(DIRECTLY_SUPPORTED_FILESYSTEM.containsKey(uri.getScheme())) {
+                        final Collection<String> plugins =
+                                
DIRECTLY_SUPPORTED_FILESYSTEM.get(uri.getScheme());
+                        throw new UnsupportedFileSystemSchemeException(
+                                String.format(
+                                        "Could not find a file system 
implementation for scheme '%s'. The scheme is "
+                                                + "directly supported by Flink 
through the following plugin%s: %s "
+                                                + "and no Hadoop file system 
to support this scheme could be loaded. Please ensure that each "
+                                                + "plugin resides within its 
own subfolder within the plugins directory. See https://ci.apache";
+                                                + 
".org/projects/flink/flink-docs-stable/ops/plugins.html for more information."

Review Comment:
   The documentation is no longer hosted at ci.apache.org, it would be good to 
directly point to the correct URL. 



-- 
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: issues-unsubscr...@flink.apache.org

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

Reply via email to