You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by zentol <gi...@git.apache.org> on 2018/02/12 12:16:01 UTC
[GitHub] flink pull request #5459: [FLINK-8475][config][docs] Integrate FS options
GitHub user zentol opened a pull request:
https://github.com/apache/flink/pull/5459
[FLINK-8475][config][docs] Integrate FS options
## What is the purpose of the change
This PR ports the fileystem ConfigConstants to `ConfigOptions` and integrates them into the configuration docs generator.
## Brief change log
* port filesystem config constants to config options
* Add missing descriptions to config options (derived from existing description/javadocs)
* integrate filesystem configuration table into `config.md`
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zentol/flink 8475_fs
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/5459.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #5459
----
commit 6238b4184295ab31933b4cc62a1f30ac11c0f09f
Author: zentol <ch...@...>
Date: 2018-01-22T15:16:02Z
[FLINK-8475][config][docs] Integrate FS options
----
---
[GitHub] flink issue #5459: [FLINK-8475][config][docs] Integrate FS options
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/5459
merging
---
[GitHub] flink pull request #5459: [FLINK-8475][config][docs] Integrate FS options
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/5459#discussion_r167592653
--- Diff: flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java ---
@@ -127,7 +130,30 @@
*/
public static final ConfigOption<String> DEFAULT_FILESYSTEM_SCHEME = ConfigOptions
.key("fs.default-scheme")
- .noDefaultValue();
+ .noDefaultValue()
+ .withDescription("The default filesystem scheme, used for paths that do not declare a scheme explicitly.");
--- End diff --
Why does this not have the whole long text that the option has in the current documentation?
---
[GitHub] flink pull request #5459: [FLINK-8475][config][docs] Integrate FS options
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/5459#discussion_r167901429
--- Diff: flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java ---
@@ -127,7 +130,30 @@
*/
public static final ConfigOption<String> DEFAULT_FILESYSTEM_SCHEME = ConfigOptions
.key("fs.default-scheme")
- .noDefaultValue();
+ .noDefaultValue()
+ .withDescription("The default filesystem scheme, used for paths that do not declare a scheme explicitly.");
--- End diff --
Ok, then I'd say merge it like this.
---
[GitHub] flink pull request #5459: [FLINK-8475][config][docs] Integrate FS options
Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on a diff in the pull request:
https://github.com/apache/flink/pull/5459#discussion_r167826760
--- Diff: flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java ---
@@ -127,7 +130,30 @@
*/
public static final ConfigOption<String> DEFAULT_FILESYSTEM_SCHEME = ConfigOptions
.key("fs.default-scheme")
- .noDefaultValue();
+ .noDefaultValue()
+ .withDescription("The default filesystem scheme, used for paths that do not declare a scheme explicitly.");
--- End diff --
Ah, because we have no default in here but isn't the `FileSystem` code using that as the default fallback?
---
[GitHub] flink pull request #5459: [FLINK-8475][config][docs] Integrate FS options
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5459#discussion_r167606303
--- Diff: flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java ---
@@ -127,7 +130,30 @@
*/
public static final ConfigOption<String> DEFAULT_FILESYSTEM_SCHEME = ConfigOptions
.key("fs.default-scheme")
- .noDefaultValue();
+ .noDefaultValue()
+ .withDescription("The default filesystem scheme, used for paths that do not declare a scheme explicitly.");
--- End diff --
IMO the missing text isn't really needed, but I'll add it back to keep this PR as a straight port of the existing docs and merge it afterwards.
---
[GitHub] flink pull request #5459: [FLINK-8475][config][docs] Integrate FS options
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5459#discussion_r167608020
--- Diff: flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java ---
@@ -127,7 +130,30 @@
*/
public static final ConfigOption<String> DEFAULT_FILESYSTEM_SCHEME = ConfigOptions
.key("fs.default-scheme")
- .noDefaultValue();
+ .noDefaultValue()
+ .withDescription("The default filesystem scheme, used for paths that do not declare a scheme explicitly.");
--- End diff --
ah I remember now, looking at the description:
[Part1]
The default filesystem scheme to be used
[Part2]
, with the necessary authority to contact, e.g. the host:port of the NameNode in the case of HDFS (if needed).
[Part3]
By default, this is set to file:/// which points to the local filesystem. This means that the local filesystem is going to be used to search for user-specified files without an explicit scheme definition.
[Part4]
This scheme is used ONLY if no other scheme is specified (explicitly) in the user-provided URI.
Part 1 and 4 are contained in the description, part 3 was left out since file:/// isn't the documented default. Only part 2 is really missing.
---
[GitHub] flink pull request #5459: [FLINK-8475][config][docs] Integrate FS options
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5459#discussion_r167831341
--- Diff: flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java ---
@@ -127,7 +130,30 @@
*/
public static final ConfigOption<String> DEFAULT_FILESYSTEM_SCHEME = ConfigOptions
.key("fs.default-scheme")
- .noDefaultValue();
+ .noDefaultValue()
+ .withDescription("The default filesystem scheme, used for paths that do not declare a scheme explicitly.");
--- End diff --
Yes, in a practical sense "file:///" is the default due to how `FileSystem` works. We can't define it as the official default at the moment because of OS compatibility, see LocalFileSystem:
```
LOCAL_URI = OperatingSystem.isWindows() ? URI.create("file:/") : URI.create("file:///");
```
Not sure whether that is really necessary though, but the scheme parsing by `Paths` is a [bit wonky on Windows](https://issues.apache.org/jira/browse/FLINK-6889).
---
[GitHub] flink pull request #5459: [FLINK-8475][config][docs] Integrate FS options
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/5459
---