[BEAM-59] Beam FileSystems: add match(), copy(), rename(), delete() utilities. #2175

peihe · 2017-03-07T02:05:43Z

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

Make sure the PR title is formatted like:
[BEAM-<Jira issue #>] Description of pull request
Make sure tests pass via mvn clean verify. (Even better, enable
Travis-CI on your fork and ensure the whole test matrix passes).
Replace <Jira issue #> in the title with the actual Jira issue
number, if there is one.
If this contribution is large, please file an Apache
Individual Contributor License Agreement.

asfbot · 2017-03-07T02:35:05Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8159/
--none--

asfbot · 2017-03-10T04:09:09Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8296/
--none--

asfbot · 2017-03-10T06:25:08Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8303/
--none--

coveralls · 2017-03-10T09:49:35Z

Coverage increased (+0.1%) to 70.311% when pulling 0bc6057 on peihe:file-system-FileSystems into 2c2424c on apache:master.

asfbot · 2017-03-10T09:49:36Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8313/

Failed Tests: 4

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-sdks-java-core: 4

--none--

peihe · 2017-03-14T09:41:08Z

R: @dhalperi
CC: @davorbonaci

asfbot · 2017-03-14T09:47:28Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8379/

Build result: FAILURE

[...truncated 553.79 KB...] at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.maven.plugin.compiler.CompilationFailureException: Compilation failure/home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall@2/runners/core-java/src/test/java/org/apache/beam/runners/core/ReduceFnTester.java:[364,25] no suitable method found for add(W) method java.util.Collection.add(org.apache.beam.runners.core.StateNamespace) is not applicable (argument mismatch; W cannot be converted to org.apache.beam.runners.core.StateNamespace) method java.util.Set.add(org.apache.beam.runners.core.StateNamespace) is not applicable (argument mismatch; W cannot be converted to org.apache.beam.runners.core.StateNamespace) at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:1029) at org.apache.maven.plugin.compiler.TestCompilerMojo.execute(TestCompilerMojo.java:170) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-03-14T09:47:07.477 [ERROR] 2017-03-14T09:47:07.477 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-03-14T09:47:07.477 [ERROR] 2017-03-14T09:47:07.477 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-03-14T09:47:07.477 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-03-14T09:47:07.478 [ERROR] 2017-03-14T09:47:07.478 [ERROR] After correcting the problems, you can resume the build with the command2017-03-14T09:47:07.478 [ERROR] mvn -rf :beam-runners-core-javachannel stoppedSetting status of f40cd72 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8379/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

asfbot · 2017-03-17T02:49:26Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8511/

Build result: FAILURE

[...truncated 557.75 KB...] at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.maven.plugin.compiler.CompilationFailureException: Compilation failure/home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/runners/core-java/src/test/java/org/apache/beam/runners/core/ReduceFnTester.java:[364,25] no suitable method found for add(W) method java.util.Collection.add(org.apache.beam.runners.core.StateNamespace) is not applicable (argument mismatch; W cannot be converted to org.apache.beam.runners.core.StateNamespace) method java.util.Set.add(org.apache.beam.runners.core.StateNamespace) is not applicable (argument mismatch; W cannot be converted to org.apache.beam.runners.core.StateNamespace) at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:1029) at org.apache.maven.plugin.compiler.TestCompilerMojo.execute(TestCompilerMojo.java:170) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-03-17T02:49:16.927 [ERROR] 2017-03-17T02:49:16.927 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-03-17T02:49:16.927 [ERROR] 2017-03-17T02:49:16.927 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-03-17T02:49:16.928 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-03-17T02:49:16.928 [ERROR] 2017-03-17T02:49:16.928 [ERROR] After correcting the problems, you can resume the build with the command2017-03-17T02:49:16.928 [ERROR] mvn -rf :beam-runners-core-javachannel stoppedSetting status of bac345c to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8511/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

…tilities.

asfbot · 2017-03-17T03:09:24Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8512/

Build result: FAILURE

[...truncated 557.79 KB...] at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.maven.plugin.compiler.CompilationFailureException: Compilation failure/home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/runners/core-java/src/test/java/org/apache/beam/runners/core/ReduceFnTester.java:[364,25] no suitable method found for add(W) method java.util.Collection.add(org.apache.beam.runners.core.StateNamespace) is not applicable (argument mismatch; W cannot be converted to org.apache.beam.runners.core.StateNamespace) method java.util.Set.add(org.apache.beam.runners.core.StateNamespace) is not applicable (argument mismatch; W cannot be converted to org.apache.beam.runners.core.StateNamespace) at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:1029) at org.apache.maven.plugin.compiler.TestCompilerMojo.execute(TestCompilerMojo.java:170) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-03-17T03:08:40.894 [ERROR] 2017-03-17T03:08:40.894 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-03-17T03:08:40.894 [ERROR] 2017-03-17T03:08:40.894 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-03-17T03:08:40.894 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-03-17T03:08:40.894 [ERROR] 2017-03-17T03:08:40.894 [ERROR] After correcting the problems, you can resume the build with the command2017-03-17T03:08:40.894 [ERROR] mvn -rf :beam-runners-core-javachannel stoppedSetting status of 7b9d764 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8512/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

coveralls · 2017-03-17T06:55:01Z

Coverage increased (+0.2%) to 70.317% when pulling a9b3c9f on peihe:file-system-FileSystems into 25b52c5 on apache:master.

asfbot · 2017-03-17T06:58:21Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8513/
--none--

peihe · 2017-03-20T01:54:50Z

PTAL

Look to me travis failing is unrelated

dhalperi · 2017-03-21T05:56:18Z

Apologies for the delay; I've been traveling.

dhalperi

Looks pretty good!

dhalperi · 2017-03-21T05:57:22Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+   * <li>{@code spec} could be a glob or a uri. {@link #match} should be able to tell and
+   * choose efficient implementations.
+   * <li>The user-provided {@code spec} might refer to files or directories. It is common that
+   * users that wish to indicate a directory will omit the trailing {@code /}, such as in a spec of


maybe hint that / is not the only delimiter, such as \ for windows or other. ?

dhalperi · 2017-03-21T05:59:12Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+            return parseScheme(spec);
+          }})
+        .toSet();
+    checkArgument(schemes.size() == 1, "Expect specs have the same scheme.");


add the set of schemes to the message for debugging?

dhalperi · 2017-03-21T05:59:26Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+          }})
+        .toSet();
+    checkArgument(schemes.size() == 1, "Expect specs have the same scheme.");
+    return getFileSystemInternal(schemes.iterator().next()).match(specs);


Iterables.getOnlyElement or something like it?

dhalperi · 2017-03-21T06:07:40Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+   */
+  public static List<MatchResult> match(List<String> specs) throws IOException {
+    checkArgument(!specs.isEmpty(), "Expect specs are not empty.");
+    Set<String> schemes = FluentIterable.from(specs)


seems like it would be good to extract this logic to a common place -- "getOnlyScheme"? This would take a list of resources and assert that they all have the same scheme.

Note that we need this here for String but also for ResourceId in all the bulk APIs below.

dhalperi · 2017-03-21T06:08:21Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+  public static void copy(
+      List<ResourceId> srcResourceIds,
+      List<ResourceId> destResourceIds,
+      MoveOptions... moveOptions) throws IOException {


MoveOptions or CopyOptions or something else?

I was following the nio interface, which uses CopyOption in both copy() and move().
https://docs.oracle.com/javase/7/docs/api/java/nio/file/CopyOption.html
(And, I also used the same options in delete()).

I think CopyOptions and RenameOptions could share many configuration options.
If we need copy specific options, we can make CopyOptions to extend MoveOptions. This interface accept both.

dhalperi · 2017-03-21T06:10:58Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+      srcToCopy = new ArrayList<>();
+      destToCopy = new ArrayList<>();
+
+      List<MatchResult> matchResults = matchResources(srcResourceIds);


extract this logic for ignoring missing files to a common place?

dhalperi · 2017-03-21T06:11:41Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+  public static void rename(
+      List<ResourceId> srcResourceIds,
+      List<ResourceId> destResourceIds,
+      MoveOptions... moveOptions) throws IOException {


RenameOptions or something else?

dhalperi · 2017-03-21T06:13:10Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalFileSystem.java

@@ -24,16 +24,17 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
+


peihe

PTAL

peihe · 2017-03-28T15:32:51Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/LocalFileSystem.java

@@ -24,16 +24,17 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
+


peihe · 2017-03-28T15:38:00Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+   * <li>{@code spec} could be a glob or a uri. {@link #match} should be able to tell and
+   * choose efficient implementations.
+   * <li>The user-provided {@code spec} might refer to files or directories. It is common that
+   * users that wish to indicate a directory will omit the trailing {@code /}, such as in a spec of


peihe · 2017-03-28T15:50:21Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+            return parseScheme(spec);
+          }})
+        .toSet();
+    checkArgument(schemes.size() == 1, "Expect specs have the same scheme.");


peihe · 2017-03-28T15:50:26Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+   */
+  public static List<MatchResult> match(List<String> specs) throws IOException {
+    checkArgument(!specs.isEmpty(), "Expect specs are not empty.");
+    Set<String> schemes = FluentIterable.from(specs)


peihe · 2017-03-28T15:50:33Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+          }})
+        .toSet();
+    checkArgument(schemes.size() == 1, "Expect specs have the same scheme.");
+    return getFileSystemInternal(schemes.iterator().next()).match(specs);


peihe · 2017-03-28T16:16:14Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+      srcToCopy = new ArrayList<>();
+      destToCopy = new ArrayList<>();
+
+      List<MatchResult> matchResults = matchResources(srcResourceIds);


peihe · 2017-03-28T16:26:44Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+  public static void copy(
+      List<ResourceId> srcResourceIds,
+      List<ResourceId> destResourceIds,
+      MoveOptions... moveOptions) throws IOException {


I was following the nio interface, which uses CopyOption in both copy() and move().
https://docs.oracle.com/javase/7/docs/api/java/nio/file/CopyOption.html
(And, I also used the same options in delete()).

I think CopyOptions and RenameOptions could share many configuration options.
If we need copy specific options, we can make CopyOptions to extend MoveOptions. This interface accept both.

peihe · 2017-03-28T16:26:51Z

sdks/java/core/src/main/java/org/apache/beam/sdk/io/FileSystems.java

+  public static void rename(
+      List<ResourceId> srcResourceIds,
+      List<ResourceId> destResourceIds,
+      MoveOptions... moveOptions) throws IOException {


jbonofre · 2017-03-28T16:45:16Z

That's awesome ! I gonna make try and experiment a filesystem (HDFS/S3 first). Thanks again !

asfbot · 2017-03-28T16:47:40Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8860/
--none--

dhalperi · 2017-03-28T23:38:38Z

LGTM once green. (Please self-merge.)

Right now, seems like Findbugs is finding null pointer exceptions:

2017-03-28T16:36:28.974 [INFO] --- findbugs-maven-plugin:3.0.4:check (default) @ beam-sdks-java-core ---
2017-03-28T16:36:29.015 [INFO] BugInstance size is 3
2017-03-28T16:36:29.019 [INFO] Error size is 0
2017-03-28T16:36:29.044 [INFO] Total bugs: 3
2017-03-28T16:36:29.049 [INFO] resourceId must be non-null but is marked as nullable [org.apache.beam.sdk.io.FileSystems$1] At FileSystems.java:[line 130] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
2017-03-28T16:36:29.049 [INFO] matchResult must be non-null but is marked as nullable [org.apache.beam.sdk.io.FileSystems$4] At FileSystems.java:[line 271] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
2017-03-28T16:36:29.049 [INFO] resourceId must be non-null but is marked as nullable [org.apache.beam.sdk.io.FileSystems$5] At FileSystems.java:[line 329] NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE
2017-03-28T16:36:29.049 [INFO]

coveralls · 2017-03-30T06:11:11Z

Coverage increased (+0.1%) to 70.268% when pulling 89a11b6 on peihe:file-system-FileSystems into 25b52c5 on apache:master.

asfbot · 2017-03-30T06:12:14Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8941/

Failed Tests: 4

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-sdks-java-core: 4

--none--

coveralls · 2017-03-30T07:32:09Z

Coverage increased (+0.2%) to 70.322% when pulling 7ab3ea6 on peihe:file-system-FileSystems into 25b52c5 on apache:master.

asfbot · 2017-03-30T07:36:12Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8946/
--none--

peihe mentioned this pull request Mar 7, 2017

[BEAM-1441] BeamFileSystem implementation instead of IOChannelFactory #2136

Closed

4 tasks

peihe force-pushed the file-system-FileSystems branch from 5508baa to 5950663 Compare March 10, 2017 03:37

peihe force-pushed the file-system-FileSystems branch from 5950663 to 8e53b4b Compare March 10, 2017 06:20

peihe force-pushed the file-system-FileSystems branch from 8e53b4b to 0bc6057 Compare March 10, 2017 09:03

peihe force-pushed the file-system-FileSystems branch 2 times, most recently from f4c437e to bac345c Compare March 17, 2017 02:43

[BEAM-59] Beam FileSystems: add match(), copy(), rename(), delete() u…

4c6e0d0

…tilities.

peihe force-pushed the file-system-FileSystems branch from bac345c to 7b9d764 Compare March 17, 2017 02:57

fixup! address tests.

a9b3c9f

peihe force-pushed the file-system-FileSystems branch from 7b9d764 to a9b3c9f Compare March 17, 2017 06:19

dhalperi requested changes Mar 21, 2017

View reviewed changes

fixup! address comments.

f5994d4

peihe commented Mar 28, 2017

View reviewed changes

fixup! address findbugs.

89a11b6

fixup! revert some usages of Iterables.getOnlyElement().

7ab3ea6

asfgit closed this in 769398e Mar 30, 2017

peihe deleted the file-system-FileSystems branch August 15, 2017 09:25

[BEAM-59] Beam FileSystems: add match(), copy(), rename(), delete() utilities. #2175

[BEAM-59] Beam FileSystems: add match(), copy(), rename(), delete() utilities. #2175

Conversation

peihe commented Mar 7, 2017

asfbot commented Mar 7, 2017

asfbot commented Mar 10, 2017

asfbot commented Mar 10, 2017

coveralls commented Mar 10, 2017

asfbot commented Mar 10, 2017

Failed Tests: 4

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-sdks-java-core: 4

peihe commented Mar 14, 2017

asfbot commented Mar 14, 2017

Build result: FAILURE

asfbot commented Mar 17, 2017

Build result: FAILURE

asfbot commented Mar 17, 2017

Build result: FAILURE

coveralls commented Mar 17, 2017

asfbot commented Mar 17, 2017

peihe commented Mar 20, 2017

dhalperi commented Mar 21, 2017

dhalperi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peihe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbonofre commented Mar 28, 2017

asfbot commented Mar 28, 2017

dhalperi commented Mar 28, 2017 • edited Loading

coveralls commented Mar 30, 2017

asfbot commented Mar 30, 2017

Failed Tests: 4

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-sdks-java-core: 4

coveralls commented Mar 30, 2017

asfbot commented Mar 30, 2017

dhalperi commented Mar 28, 2017 •

edited

Loading