You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by StephanEwen <gi...@git.apache.org> on 2017/10/06 10:11:03 UTC

[GitHub] flink pull request #4781: [FLINK-7768] [core] Load File Systems via Java Ser...

GitHub user StephanEwen opened a pull request:

    https://github.com/apache/flink/pull/4781

    [FLINK-7768] [core] Load File Systems via Java Service abstraction

    **This is based on #4780 , so only the latest commit is relevant**

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/StephanEwen/incubator-flink fs_services

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/4781.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 #4781
    
----
commit 7843c2ffb44f99967dc71746ac1c79b04a74fe80
Author: Stephan Ewen <se...@apache.org>
Date:   2017-10-05T09:27:48Z

    [FLINK-7766] [file system sink] Drop obsolete reflective hflush calls
    
    This was done reflectively before for Hadoop 1 compatibility.
    Since Hadoop 1 is no longer supported, this is obsolete now.

commit 09a3bf41a0215ff1d7d9c6b60b527a671aee448a
Author: Stephan Ewen <se...@apache.org>
Date:   2017-10-05T09:26:13Z

    [FLINK-7767] [file system sinks] Avoid loading Hadoop conf dynamically at runtime

commit 119db226bc915a84e9e0f007e09a55eb8bda8395
Author: Stephan Ewen <se...@apache.org>
Date:   2017-10-05T16:12:21Z

    [FLINK-7768] [core] Load File Systems via Java Service abstraction
    
    This changes the discovery mechanism of file from static class name configurations
    to a service mechanism (META-INF/services).
    
    As part of that, it factors HDFS and MapR FS implementations into separate modules.
    
    With this change, users can add new filesystem implementations and make them available
    by simply adding them to the class path.

----


---

[GitHub] flink issue #4781: [FLINK-7768] [core] Load File Systems via Java Service ab...

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the issue:

    https://github.com/apache/flink/pull/4781
  
    The changes look good! 👍 I had some comments about POMs, optional and shading but those got resolved "offline". I had one comment about a test name but feel free to merge.


---

[GitHub] flink pull request #4781: [FLINK-7768] [core] Load File Systems via Java Ser...

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4781#discussion_r143174730
  
    --- Diff: flink-dist/pom.xml ---
    @@ -205,6 +205,32 @@ under the License.
     			</exclusions>
     		</dependency>
     
    +		<!-- Default file system support -->
    +
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-hadoop-fs</artifactId>
    +			<version>${project.version}</version>
    +			<exclusions>
    +				<exclusion>
    +					<groupId>org.apache.flink</groupId>
    +					<artifactId>flink-shaded-hadoop2</artifactId>
    +				</exclusion>
    +			</exclusions>
    +		</dependency>
    +		
    +		<dependency>
    +			<groupId>org.apache.flink</groupId>
    +			<artifactId>flink-mapr-fs</artifactId>
    +			<version>${project.version}</version>
    --- End diff --
    
    This works because flink-shaded-hadoop2 is a transitive optional dependency? We don't exclude it explicitly here.


---

[GitHub] flink pull request #4781: [FLINK-7768] [core] Load File Systems via Java Ser...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/4781


---

[GitHub] flink pull request #4781: [FLINK-7768] [core] Load File Systems via Java Ser...

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4781#discussion_r143171918
  
    --- Diff: flink-filesystems/flink-mapr-fs/src/test/java/org/apache/flink/runtime/fs/maprfs/FileSystemAccessTest.java ---
    @@ -0,0 +1,42 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.runtime.fs.maprfs;
    +
    +import org.apache.flink.core.fs.FileSystem;
    +import org.apache.flink.core.fs.Path;
    +import org.apache.flink.util.TestLogger;
    +
    +import org.junit.Test;
    +
    +import static org.junit.Assert.assertEquals;
    +
    +/**
    + * This test checks that the file system is properly accessible through the
    + * service loading abstraction.
    + */
    +public class FileSystemAccessTest extends TestLogger {
    --- End diff --
    
    nit: have MapR in the test name


---