[BEAM-59] Beam FileSystem.setDefaultConfig: remove scheme from the signature. #1826
Conversation
|
Refer to this link for build results (access rights to CI server needed): |
peihe
force-pushed
the
file-system-set-default
branch
from
a5193ae
to
d08879f
Compare
|
Refer to this link for build results (access rights to CI server needed): Failed Tests: 1beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-sdks-java-core: 1--none-- |
peihe
force-pushed
the
file-system-set-default
branch
from
d08879f
to
974975e
Compare
| @@ -108,7 +94,8 @@ static PipelineOptions getDefaultConfig(String scheme) { | |||
| static FileSystem getFileSystemInternal(URI uri) { | |||
| String lowerCaseScheme = (uri.getScheme() != null | |||
| ? uri.getScheme().toLowerCase() : LocalFileSystemRegistrar.LOCAL_FILE_SCHEME); | |||
| return getRegistrarInternal(lowerCaseScheme).fromOptions(getDefaultConfig(lowerCaseScheme)); | |||
| return getRegistrarInternal(lowerCaseScheme) | |||
| .fromOptions(checkNotNull(defaultConfig, "defaultConfig")); | |||
There was a problem hiding this comment.
Should the error message be improved? It seems like if this fails the resolution is complex. Like a runner should have called FileSystems.setDefaultConfigInWorkers? Or the user needs to register a config for the specific URI?
Probably the URI should be in the error message too.
There was a problem hiding this comment.
Done
Didn't include the URI, since the check fail will happen for any scheme for now.
|
Refer to this link for build results (access rights to CI server needed): |
|
PTAL |
| } | ||
|
|
||
| /** | ||
| * Internal method to get {@link FileSystem} for {@code spec}. | ||
| */ | ||
| @VisibleForTesting | ||
| static FileSystem getFileSystemInternal(URI uri) { | ||
| checkNotNull( |
There was a problem hiding this comment.
nit: checkState(defaultConfig != null to throw the proper exception type. But, I can fix this in the merge if LGTY.
|
Refer to this link for build results (access rights to CI server needed): |
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.