[BEAM-646] Always expand getInputs, getOutputs in AppliedPTransform #1735

tgroh · 2017-01-05T01:52:58Z

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.

Removes the ability for a runner to access compressed inputs and outputs from an AppliedPTransform.

asfbot · 2017-01-05T02:06:21Z

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

Build result: FAILURE

[...truncated 10462 lines...] at hudson.remoting.UserRequest.perform(UserRequest.java:153) at hudson.remoting.UserRequest.perform(UserRequest.java:50) at hudson.remoting.Request$2.run(Request.java:332) at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.maven.plugin.MojoFailureException: You have 1 Checkstyle violation. at org.apache.maven.plugin.checkstyle.CheckstyleViolationCheckMojo.execute(CheckstyleViolationCheckMojo.java:588) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-01-05T02:06:18.062 [ERROR] 2017-01-05T02:06:18.062 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-01-05T02:06:18.062 [ERROR] 2017-01-05T02:06:18.062 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-01-05T02:06:18.062 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-01-05T02:06:18.062 [ERROR] 2017-01-05T02:06:18.062 [ERROR] After correcting the problems, you can resume the build with the command2017-01-05T02:06:18.062 [ERROR] mvn -rf :beam-runners-flink_2.10channel stoppedSetting status of efccd93 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6418/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

asfbot · 2017-01-05T02:28:06Z

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

Build result: FAILURE

[...truncated 11462 lines...] at hudson.remoting.UserRequest.perform(UserRequest.java:153) at hudson.remoting.UserRequest.perform(UserRequest.java:50) at hudson.remoting.Request$2.run(Request.java:332) at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.maven.plugin.MojoFailureException: You have 1 Checkstyle violation. at org.apache.maven.plugin.checkstyle.CheckstyleViolationCheckMojo.execute(CheckstyleViolationCheckMojo.java:588) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-01-05T02:28:03.511 [ERROR] 2017-01-05T02:28:03.511 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-01-05T02:28:03.511 [ERROR] 2017-01-05T02:28:03.511 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-01-05T02:28:03.511 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-01-05T02:28:03.511 [ERROR] 2017-01-05T02:28:03.511 [ERROR] After correcting the problems, you can resume the build with the command2017-01-05T02:28:03.511 [ERROR] mvn -rf :beam-runners-sparkchannel stoppedSetting status of 0cde110 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6420/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

asfbot · 2017-01-05T19:00:02Z

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

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

org.apache.beam.examples.WordCountIT.testE2EWordCount

--none--

asfbot · 2017-01-05T22:00:05Z

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

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

org.apache.beam.examples.WindowedWordCountIT.testWindowedWordCountInBatch

--none--

asfbot · 2017-01-06T19:29:37Z

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

asfbot · 2017-01-17T20:04:57Z

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

Build result: FAILURE

[...truncated 6068 lines...] at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) 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/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowPipelineTranslator.java:[155,11] illegal start of expression at org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute(AbstractCompilerMojo.java:972) at org.apache.maven.plugin.compiler.CompilerMojo.execute(CompilerMojo.java:129) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-01-17T20:04:47.263 [ERROR] 2017-01-17T20:04:47.263 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-01-17T20:04:47.263 [ERROR] 2017-01-17T20:04:47.263 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-01-17T20:04:47.263 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-01-17T20:04:47.263 [ERROR] 2017-01-17T20:04:47.263 [ERROR] After correcting the problems, you can resume the build with the command2017-01-17T20:04:47.263 [ERROR] mvn -rf :beam-runners-google-cloud-dataflow-javachannel stoppedSetting status of 0d91501 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6590/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

asfbot · 2017-01-17T22:27:33Z

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

tgroh · 2017-01-18T00:45:05Z

R: @kennknowles
R: @tweise (for Apex changes) @aljoscha (for Flink changes) @amitsela (for Spark changes)

amitsela

LGTM, added a small comment.

amitsela · 2017-01-18T11:45:33Z

runners/spark/src/main/java/org/apache/beam/runners/spark/translation/EvaluationContext.java

    checkArgument(currentTransform != null && currentTransform.getTransform() == transform,
        "can only be called with current transform");
    @SuppressWarnings("unchecked")
-    T input = (T) currentTransform.getInput();
+    T input = (T) Iterables.getOnlyElement(currentTransform.getInputs()).getValue();


Couldn't you simply call getInput() instead of currentTransform.getInputs() and it will validate so you could drop the checkArguemnts as well. True for getOnlyOutput() as well.

asfbot · 2017-01-18T17:01:18Z

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

asfbot · 2017-01-18T17:21:21Z

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

asfbot · 2017-01-18T17:24:46Z

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

coveralls · 2017-01-18T22:56:40Z

Coverage increased (+0.006%) to 69.949% when pulling 936037e on tgroh:no_compressed_inputs into d135493 on apache:master.

asfbot · 2017-01-18T22:58:14Z

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

dhalperi · 2017-01-20T07:11:08Z

...s/apex/src/main/java/org/apache/beam/runners/apex/translation/ParDoBoundMultiTranslator.java

      } else {
        int portIndex = 0;
        for (TupleTag<?> tag : transform.getSideOutputTags().getAll()) {
-          if (tag == outputEntry.getKey()) {
-            ports.put(outputEntry.getValue(), operator.sideOutputPorts[portIndex]);
+          if (tag == output.getTag()) {


dhalperi · 2017-01-20T07:11:49Z

...s/apex/src/main/java/org/apache/beam/runners/apex/translation/ParDoBoundMultiTranslator.java

+          output.getValue(),
+          output.getValue().getClass().getSimpleName());
+      PCollection<?> pc = (PCollection<?>) output.getValue();
+      if (output.getTag () == transform.getMainOutputTag()) {


extra space. also == suspicion again.

coveralls · 2017-01-20T17:44:26Z

Coverage decreased (-0.07%) to 69.869% when pulling 3836602 on tgroh:no_compressed_inputs into d135493 on apache:master.

asfbot · 2017-01-20T17:45:34Z

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

Never provide unexpanded input or output types from AppliedPTransform, as they are not part of the model. Update all runners to obtain appropriate PValues from the expansions.

asfbot · 2017-01-21T01:11:42Z

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

coveralls · 2017-01-21T01:11:42Z

Coverage increased (+0.02%) to 69.798% when pulling 4ba7b7f on tgroh:no_compressed_inputs into f799a57 on apache:master.

tweise

ApexRunner changes LGTM with two minor comments.

tweise · 2017-01-23T04:35:42Z

runners/apex/src/main/java/org/apache/beam/runners/apex/translation/TranslationContext.java

  }

-  public <OutputT extends POutput> OutputT getOutput() {
-    return (OutputT) getCurrentTransform().getOutput();
+  public List<TaggedPValue> getInput() {


getInputs (plural)

Done. Also renamed getOnlyInput to getInput

tweise · 2017-01-23T04:35:55Z

runners/apex/src/main/java/org/apache/beam/runners/apex/translation/TranslationContext.java

+    return Iterables.getOnlyElement(getCurrentTransform().getInputs()).getValue();
+  }
+
+  public List<TaggedPValue> getOutput() {


Done. Also renamed getOnlyOutput to getOutput.

aljoscha · 2017-01-23T16:48:49Z

...rc/main/java/org/apache/beam/runners/flink/translation/FlinkStreamingTranslationContext.java

+    return (T) Iterables.getOnlyElement(currentTransform.getInputs()).getValue();
+  }
+
+  public <T extends PInput> List<TaggedPValue> getInput(PTransform<T, ?> transform) {


Should this be getInputs()?

Same holds for getOutput() and the corresponding methods in FlinkBatchTranslationContext.

Done. Renamed multi-input/output to getInputs() and getOutputs() and singular into getInput() and getOuput()

aljoscha · 2017-01-23T16:50:04Z

Changes look good for the Flink Runner. 👍

I had a comment about the methods names (missing plural s).

tgroh

I've just performed a global replace of getInput -> getInputs, getOuput -> getOutputs, getOnlyInput -> getInput and getOnlyOutput to getOutput.

@kennknowles runner authors are all happy, PTAL

tgroh · 2017-01-23T17:59:04Z

...rc/main/java/org/apache/beam/runners/flink/translation/FlinkStreamingTranslationContext.java

+    return (T) Iterables.getOnlyElement(currentTransform.getInputs()).getValue();
+  }
+
+  public <T extends PInput> List<TaggedPValue> getInput(PTransform<T, ?> transform) {


Done. Renamed multi-input/output to getInputs() and getOutputs() and singular into getInput() and getOuput()

tgroh · 2017-01-23T18:01:18Z

runners/apex/src/main/java/org/apache/beam/runners/apex/translation/TranslationContext.java

  }

-  public <OutputT extends POutput> OutputT getOutput() {
-    return (OutputT) getCurrentTransform().getOutput();
+  public List<TaggedPValue> getInput() {


Done. Also renamed getOnlyInput to getInput

tgroh · 2017-01-23T18:01:29Z

runners/apex/src/main/java/org/apache/beam/runners/apex/translation/TranslationContext.java

+    return Iterables.getOnlyElement(getCurrentTransform().getInputs()).getValue();
+  }
+
+  public List<TaggedPValue> getOutput() {


Done. Also renamed getOnlyOutput to getOutput.

asfbot · 2017-01-23T20:16:01Z

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

kennknowles

LGTM!

tgroh · 2017-01-23T23:10:19Z

Retest this please

coveralls · 2017-01-23T23:41:46Z

Coverage increased (+0.0002%) to 69.775% when pulling 0c5a771 on tgroh:no_compressed_inputs into f799a57 on apache:master.

asfbot · 2017-01-23T23:41:48Z

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

tgroh force-pushed the no_compressed_inputs branch from efccd93 to 0cde110 Compare January 5, 2017 02:09

tgroh force-pushed the no_compressed_inputs branch from 0cde110 to 1c39bb2 Compare January 5, 2017 18:34

tgroh force-pushed the no_compressed_inputs branch from 1c39bb2 to ff4b1d1 Compare January 5, 2017 21:32

tgroh force-pushed the no_compressed_inputs branch from ff4b1d1 to 67de755 Compare January 6, 2017 18:59

tgroh changed the title ~~Always expand getInputs, getOutputs in AppliedPTransform~~ [BEAM-646] Always expand getInputs, getOutputs in AppliedPTransform Jan 17, 2017

tgroh force-pushed the no_compressed_inputs branch from 67de755 to 0d91501 Compare January 17, 2017 19:53

tgroh force-pushed the no_compressed_inputs branch from 0d91501 to f8312b8 Compare January 17, 2017 21:59

amitsela reviewed Jan 18, 2017

View reviewed changes

tgroh force-pushed the no_compressed_inputs branch from 0300bed to 43d59f9 Compare January 18, 2017 17:19

tgroh force-pushed the no_compressed_inputs branch from 43d59f9 to 4f96900 Compare January 18, 2017 17:23

tgroh force-pushed the no_compressed_inputs branch from 4f96900 to 936037e Compare January 18, 2017 22:25

dhalperi reviewed Jan 20, 2017

View reviewed changes

tgroh added 2 commits January 20, 2017 16:12

Always expand in AppliedPTransform

efbdf3c

Never provide unexpanded input or output types from AppliedPTransform, as they are not part of the model. Update all runners to obtain appropriate PValues from the expansions.

fixup! Always expand in AppliedPTransform

8e2fae3

fixup! Always expand in AppliedPTransform

4ba7b7f

tgroh force-pushed the no_compressed_inputs branch from 3836602 to 4ba7b7f Compare January 21, 2017 00:12

tweise reviewed Jan 23, 2017

View reviewed changes

aljoscha reviewed Jan 23, 2017

View reviewed changes

fixup! Always expand in AppliedPTransform

0c5a771

tgroh commented Jan 23, 2017

View reviewed changes

kennknowles approved these changes Jan 23, 2017

View reviewed changes

asfgit closed this in f15b52f Jan 24, 2017

[BEAM-646] Always expand getInputs, getOutputs in AppliedPTransform #1735

[BEAM-646] Always expand getInputs, getOutputs in AppliedPTransform #1735

Conversation

tgroh commented Jan 5, 2017 • edited

asfbot commented Jan 5, 2017

Build result: FAILURE

asfbot commented Jan 5, 2017

Build result: FAILURE

asfbot commented Jan 5, 2017

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

asfbot commented Jan 5, 2017

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-examples-java: 1

asfbot commented Jan 6, 2017

asfbot commented Jan 17, 2017

Build result: FAILURE

asfbot commented Jan 17, 2017

tgroh commented Jan 18, 2017

amitsela left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfbot commented Jan 18, 2017

asfbot commented Jan 18, 2017

asfbot commented Jan 18, 2017

coveralls commented Jan 18, 2017

asfbot commented Jan 18, 2017

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

coveralls commented Jan 20, 2017

asfbot commented Jan 20, 2017

asfbot commented Jan 21, 2017

coveralls commented Jan 21, 2017

tweise 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

aljoscha commented Jan 23, 2017

tgroh 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

asfbot commented Jan 23, 2017

kennknowles left a comment

Choose a reason for hiding this comment

tgroh commented Jan 23, 2017

coveralls commented Jan 23, 2017

asfbot commented Jan 23, 2017

tgroh commented Jan 5, 2017 •

edited