Interactive runner and the corresponding tests #5818

qinyeli · 2018-06-28T19:52:48Z

Added Beam Python Interactive Runner, including

The runner itself /sdks/python/apache_beam/runners/interactive/*
The corresponding unittest
A Jupyter notebook demo

Design doc: s.apache.org/interactive-beam
YouTube demo: https://youtu.be/c5CjA1e3Cqw

R: @robertwb
CC: @ananvay

Follow this checklist to help us incorporate your contribution quickly and easily:

Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

It will help us expedite review of your Pull Request if you tag someone (e.g. @username) to look at it.

Post-Commit Tests Status (on master branch)

Lang	Apex	Dataflow	Flink	Gearpump	Spark
Go	---	---	---	---	---
Java
Python	---		---	---	---

qinyeli · 2018-06-28T23:50:32Z

run test

pabloem · 2018-06-29T00:23:42Z

This is very cool! you can test the cython failure by running tox -e py27-cython from within sdks/python. In case you're looking for something like that : )

qinyeli · 2018-06-29T01:29:05Z

Thanks, Pablo! I was exactly trying to figure out how to run test locally.

qinyeli · 2018-07-03T21:40:58Z

@charlesccychen

robertwb

Neat stuff.

robertwb · 2018-07-06T19:50:46Z

sdks/python/setup.py

    'crcmod>=1.7,<2.0',
    'dill==0.2.6',
    'fastavro==0.19.7',
+    'graphviz==0.8.3',


Please don't pin unless you need to.

graphviz is imported in display_graph.py to construct the DOT representation of the pipeline. Don't we need to specify all required modules here?

I think @robertwb is suggesting to remove the "==0.8.3" qualifier.

robertwb · 2018-07-06T19:53:12Z

sdks/python/apache_beam/runners/interactive/display_graph.py

+  """Given vertices, edges and their attributes, construct graphviz.Digraph.
+
+  Args:
+    vertex_dict: (dict(str->(dict(str->str))) dict mapping vertex


Suggestion: Use standard notation for typing information (https://www.python.org/dev/peps/pep-0484/). Similarly elsewhere.

It looks very neat, but it seems this kind of annotation is not used anywhere in apache_beam codebase (searched "from __future__ import annotations")

Would you like me to use annotations from module typehints like @with_input_types @with_output_types or something like ~apache_beam.runners.runner.PipelineRunner instead?

robertwb · 2018-07-06T19:54:55Z

sdks/python/apache_beam/runners/interactive/display_graph.py

+    def is_top_level_transform(transform):
+      return transform.unique_name and '/' not in transform.unique_name
+
+    def leaf_transforms(parent_id):


Maybe call this leaf_transform_ids?

robertwb · 2018-07-06T20:00:24Z

sdks/python/apache_beam/runners/interactive/display_graph.py

+               pcollection_stats=None):
+    """Constructor of PipelineGraph.
+
+    All fields except for pipeline_proto should be left as None unless used by


Rather than taking interactive-specific parameters here, take a map of {pcollections,ptransforms} -> extra attributes. The interactive runner would then populate these according to its notion of cached, etc. but other runners could highlight different data.

robertwb · 2018-07-06T20:01:44Z

sdks/python/apache_beam/runners/interactive/interactive_runner.py

+from apache_beam.runners.interactive import display_manager
+from apache_beam.transforms import combiners
+
+SAMPLE_SIZE = 8


This constant deserves a comment.

robertwb · 2018-07-06T20:02:33Z

sdks/python/apache_beam/runners/interactive/interactive_runner.py

+    self._cache_manager.cleanup()
+
+  def apply(self, transform, pvalueish):
+    # TODO(qinyeli): Remove runner interception of apply.


Maybe phrase as "remove once runner interception of apply is gone" with a(n existing?) JIRA?

robertwb · 2018-07-06T20:05:00Z

sdks/python/apache_beam/runners/interactive/interactive_runner.py

+    # pylint: disable=import-error
+    from apache_beam.pipeline import Pipeline
+
+    # When possible, invoke a round trip through the runner API.


Is this still need (or desirable)?

You are right.. This is not desirable. Thanks!

robertwb · 2018-07-06T20:10:02Z

sdks/python/apache_beam/runners/interactive/interactive_runner.py

+    return self._pipeline_info.derivation(pcoll_id).cache_label()
+
+  def wait_until_finish(self):
+    return


Add a comment as to why we can do this.

robertwb · 2018-07-06T20:10:36Z

sdks/python/apache_beam/runners/interactive/interactive_runner_test.py

+
+  def test_basic(self):
+    # TODO(qinyeli) remove explicitly overriding underlying runner
+    # once interactive_runner works with FnAPI mode


Could you clarify what the issue is? This will make tests pass, but be broken for users.

qinyeli · 2018-07-10T00:57:32Z

Run Python PreCommit

charlesccychen

Thanks! This LGTM, after the remaining minor comments.

charlesccychen · 2018-07-10T04:22:38Z

sdks/python/setup.py

    'oauth2client>=2.0.1,<5',
    # grpcio 1.8.1 and above requires protobuf 3.5.0.post1.
    'protobuf>=3.5.0.post1,<4',
+    'pydot>=1.2.0,<=1.2.4',


Let's just do <1.3 for now.

charlesccychen · 2018-07-10T04:27:04Z

sdks/python/apache_beam/runners/interactive/interactive_runner.py

+
+
+class Derivation(object):
+  """Helper for PipelineInfo."""


Can you add a more descriptive comment?

charlesccychen · 2018-07-10T04:27:27Z

sdks/python/apache_beam/runners/interactive/interactive_runner.py

+    return str(self.json())
+
+
+class SafeFastPrimitivesCoder(coders.Coder):


Can you add a comment for why this extra quote / unquote step is needed?

charlesccychen · 2018-07-10T04:28:47Z

sdks/python/apache_beam/runners/interactive/pipeline_graph.py

+      vertex_dict[transform.unique_name] = {}
+
+      for pcoll_id in transform.outputs.values():
+


Remove extra newline.

qinyeli · 2018-07-10T20:36:37Z

Would you like me to squash the commits into one? @charlesccychen

charlesccychen · 2018-07-10T23:33:37Z

Run Python PostCommit

charlesccychen · 2018-07-10T23:33:41Z

Thanks @qinyeli! I will merge after tests pass.

robertwb · 2018-07-11T21:18:18Z

sdks/python/apache_beam/runners/interactive/interactive_runner.py

+      self._desired_cache_labels = set()
+    print('Running...')
+
+    # When possible, invoke a round trip through the runner API.


This adds a new subproject that exports a new shadow jar that includes the job server and all transitive dependencies. It also includes a runShadow task that can be used for local debugging via gradle.

Add game_stats_it_test.py Fix datasets in *_it_tests not deleted Relocate *_it_tests in the same directory as the code they test

Support date type in comparison expression Test date type comparison at the DSL level

* Cythonizing a few more hot calls.

Adding combine URNs based on the portable combines doc: https://s.apache.org/beam-runner-api-combine-model I added a new URN to the doc, noticed it already existed as an old URN. For now I will simply be repurposing the URN so I only changed the comment. Also updated the comments for the other Combine URNs to make them more consistent with the other comments and describe what the expected payloads are for them.

Also changed the enum numbers for the existing elements in the Composites enum. Normally this would be bad for protos (reserving the removed enum value is preferable), but in this case the URN enums being changed are never actually used in any proto messages; They're only there for devs to use, so this won't affect anything.

Adding implementations for the components represented by the Combine URNs described in the portable combines doc: https://s.apache.org/beam-runner-api-combine-model This is just an initial implementation so it's lacking one major feature which is an optimization in the Precombine. Planning to add that right away, but want to get this in first.

report

This is necessary in order to use filtering based on touched files. In the ghprb plugin all trigger criteria is evaluated together, which means that include filters are also applied to phrase triggering. See: jenkinsci/ghprb-plugin#678

…ings FindBugs annotations are a mess. We want to eventually deprecate our own usage of FindBugs, but we still need to deal with the annotations because our dependencies (notable byte-buddy) use the annotations without providing the dependency. This causes compile warnings such as: byte-buddy-1.8.11.jar(/net/bytebuddy/description/type/TypeList.class): warning: Cannot find annotation method 'value()' in type 'SuppressFBWarnings': class file for edu.umd.cs.findbugs.annotations.SuppressFBWarnings not found To fix these warnings, we need to include a findbugs-annotations dependency during compile which includes the SuppressFBWarnings annotation. The dependency options we currently have are: * com.google.code.findbugs:annotations:3.0.1 * Includes @SuppressFBWarnings, but licensed under LGPL * com.github.stephenc.findbugs:findbugs-annotations:1.3.9-1 * Licensed under Apache 2.0, but doesn't include @SuppressFBWarnings It is ok to use LGPL components during the build, as long as they don't make it into the distribution. See: * https://www.apache.org/legal/resolved.html#prohibited * https://www.apache.org/legal/resolved.html#optional

Note that I updated the "TIMESTAMP" coder which was meant to support timers with an updated "TIMER" coder based upon https://s.apache.org/beam-portability-timers

… ready

This is a follow-up to PR #5757 which splits the pre-commit jobs and updates the naming. The GradleBuild suffix was dropped since we no longer run Maven variants.

This is a follow-up to PR #5757 which splits the pre-commit jobs and updates the naming. The GradleBuild suffix was dropped since we no longer run Maven variants. The *_Cron versions of the job run on a cron schedule and on pushes to master.

* Pin pytz version in setup.py

* improvements to fanout

The exclusions here are based on examining how other Apache projects add license headers for various file types. In particular, Apache Zepplin seemed to be a good example [1], and in other cases doing a search across Apache GitHub [2]. [1] https://github.com/apache/zeppelin/tree/master/docs [2] https://github.com/search?utf8=%E2%9C%93&q=%22See+the+License+for+the+specific+language+governing+permissions%22+user%3Aapache+extension%3Ayml&type=Code&ref=advsearch&l=&l=

charlesccychen self-requested a review July 3, 2018 22:34

robertwb reviewed Jul 6, 2018

View reviewed changes

charlesccychen reviewed Jul 10, 2018

View reviewed changes

robertwb approved these changes Jul 11, 2018

View reviewed changes

bsidhom and others added 18 commits July 11, 2018 17:40

[BEAM-4615] Flink job server wrapper and shadow jar

aed90e6

This adds a new subproject that exports a new shadow jar that includes the job server and all transitive dependencies. It also includes a runShadow task that can be used for local debugging via gradle.

[BEAM-4333] Add integration tests for python mobile games (#5747)

b943417

Add game_stats_it_test.py Fix datasets in *_it_tests not deleted Relocate *_it_tests in the same directory as the code they test

[BEAM-4602][BEAM-4598] Date type comparison

0e344e1

Support date type in comparison expression Test date type comparison at the DSL level

Cythonizing a few hot calls (#5733)

cf4f255

* Cythonizing a few more hot calls.

[SQL] Don't wait on Unbounded PCollections

543bd3d

Quick fix, removing commented out code.

d3caef3

[BEAM-4302] Add Jenkins job to check Beam dependencies and generate

cc60ad8

report

improvements to fanout

742846b

Fixes Flink ValidatesRunner and a compile error

c7744da

Filter pre-commit triggering based on touched files

0794a6a

Rename setPostCommit to setAutoJob

aee5476

Removing debug string from UserScore.java

64d9c20

Add checkstyle validation to ban usage of @SuppressFBWarnings.

119848e

xinyuiscool and others added 28 commits July 11, 2018 17:40

Remove unused previous names

1ce5f40

Apply rename of JobBuilder to Samza VR job

7135a19

Fix maven build part 9

3fdc5ea

Fix ErrorProne warnings in sdks-java-core project

fa15775

Enable ErrorProne analysis for all Java projects.

cc294d5

Enable failOnWarnings for all projects.

a120e12

Revert "improvements to fanout"

85b36e7

Add bundle-ready SQL shell using application plugin

29008b7

Reduce stdout spam from BeamModulePlugin

7303b90

Run gofmt

51e16b8

[BEAM-4661] Define well known timer URN.

632eb5e

Note that I updated the "TIMESTAMP" coder which was meant to support timers with an updated "TIMER" coder based upon https://s.apache.org/beam-portability-timers

Use ByteBuddy to generate a coder class for a specific Schema.

07352d8

Subclass proper generic type, removing need for extra typecast.

5247eed

Address code-review comments.

3941c92

[BEAM-3261] Fix pipeline failures by waiting for logging server to be…

f04c22b

… ready

[BEAM-4645] Cleanup docker images on failure

4fcab7f

Rename refactored Precommit source files to remove GradleBuild suffix.

ed336af

This is a follow-up to PR #5757 which splits the pre-commit jobs and updates the naming. The GradleBuild suffix was dropped since we no longer run Maven variants.

Adding flinkMarterUrl as an option

7e35a49

Improve BigQuery write integration tests and support TIMESTAMP SQL type

4e3a412

Fix accidental license header javadoc comments

1ee878d

Fix HTML error in ApproximateQuantiles

62fa7c3

[BEAM-4606] Pin pytz version (#5751)

242b33f

* Pin pytz version in setup.py

Increase website job timeout to 100 by default

731df16

[BEAM-2810] use fastavro-blocks in Avro IO

1ee7313

improvements to fanout (#5800)

ec451e1

* improvements to fanout

Interactive runner and the corresponding tests

8e73cb1

qinyeli closed this Jul 12, 2018

charlesccychen mentioned this pull request Jul 12, 2018

Interactive runner and corresponding tests #5937

Merged

2 tasks

		return str(self.json())


		class SafeFastPrimitivesCoder(coders.Coder):

		vertex_dict[transform.unique_name] = {}

		for pcoll_id in transform.outputs.values():

Interactive runner and the corresponding tests #5818

Interactive runner and the corresponding tests #5818

Uh oh!

Conversation

qinyeli commented Jun 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

R: @robertwb CC: @ananvay

Post-Commit Tests Status (on master branch)

Uh oh!

qinyeli commented Jun 28, 2018

Uh oh!

pabloem commented Jun 29, 2018

Uh oh!

qinyeli commented Jun 29, 2018

Uh oh!

qinyeli commented Jul 3, 2018

Uh oh!

robertwb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qinyeli Jul 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qinyeli commented Jul 10, 2018

Uh oh!

charlesccychen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qinyeli commented Jul 10, 2018

Uh oh!

charlesccychen commented Jul 10, 2018

Uh oh!

charlesccychen commented Jul 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

qinyeli commented Jun 28, 2018 •

edited

Loading

R: @robertwb
CC: @ananvay

qinyeli Jul 6, 2018 •

edited

Loading