[BEAM-6619] [BEAM-6593] Add example integration tests to postcommit #8076

Juta · 2019-03-18T08:47:24Z

This is is part of a series of PRs with goal to make Apache Beam PY3 compatible. The proposal with the outlined approach has been documented here: https://s.apache.org/beam-python-3.

This PR adds a more integration tests to the postcommit jobs on direct and dataflow runners.

Post-Commit Tests Status (on master branch)

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

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

Juta · 2019-03-18T08:47:40Z

Run Python PostCommit

Juta · 2019-03-19T13:26:04Z

Run Python PostCommit

Juta · 2019-03-19T15:56:05Z

Run Python PostCommit

Juta · 2019-03-20T09:45:41Z

Run Python PostCommit

Juta · 2019-03-20T14:12:47Z

Run Python PostCommit

Juta · 2019-03-20T15:37:16Z

cc: @pabloem @ttanay @tvalentyn

tvalentyn

Thanks a lot, @Juta for the changes and the clean up. Left some comments, please take a look.

tvalentyn · 2019-03-20T16:22:49Z

sdks/python/apache_beam/examples/complete/game/game_stats.py


  def process(self, elem):
    try:
+      if isinstance(elem, bytes):


Can we always decode here? It is better to have a clear expectation of input arguments as much as possible on Python 3: either always encoded bytes, or always strings, but not mixing the two.

tvalentyn · 2019-03-20T16:24:01Z

sdks/python/apache_beam/examples/complete/game/leader_board.py


  def process(self, elem):
    try:
+      if isinstance(elem, bytes):


Can we always decode here? It is better to have a clear expectation of input arguments as much as possible on Python 3: either always encoded bytes, or always strings, but not mixing the two.

The IT test uses pubsub as input source while the unit test currently passes strings. That's why we cannot always decode

I am not sure I follow - could you please elaborate - what exactly in either pubsub or unit test makes it so that the input type is not consistent here?

The code for leader_board and game_stats is used both with strings https://github.com/apache/beam/blob/master/sdks/python/apache_beam/examples/complete/game/leader_board_test.py#L45 and with pubsub messages: https://github.com/apache/beam/blob/master/sdks/python/apache_beam/examples/complete/game/leader_board_it_test.py#L146.

Ok, the fix would then be to:

Expect elem to be a string.

Decode the messages as they arrive from PubSub using | 'DecodeString' >> Map(lambda b: b.decode('utf-8') in the example pipeline. Note that there are several example pipelines that will need this change.

Note that we used to have ReadStringsFromPubSub method that was deprecated:

beam/sdks/python/apache_beam/io/gcp/pubsub.py

Line 194 in a5ed104

@deprecated(since='2.7.0', extra_message='Use ReadFromPubSub instead.')

.

We may want to revisit this deprecation in the future.

@tvalentyn I applied this fix

tvalentyn · 2019-03-20T16:28:40Z

sdks/python/apache_beam/examples/complete/game/leader_board_it_test.py


  # Input event containing user, team, score, processing time, window start.
-  INPUT_EVENT = 'user1,teamA,10,%d,2015-11-02 09:09:28.224'
+  INPUT_EVENT = b'user1,teamA,10,%d,2015-11-02 09:09:28.224'


I am not sure if this should be bytes. Does self.pub_client.publish require bytes? If so, we should encode before passing data to that method.

yes self.pub_client.publish requires bytes. In this case I think specifying the input event as bytes is what is expected because it is directly passed to the client. What do you think?

So the question is then, to the humans who read and edit this code, should INPUT_EVENT be a textual date or encoded data? I think it's easier to consider it text, up until it's time to feed it to pubsub, then when we can encode it to bytes.

I'd keep INPUT_EVENT as is change publishing to:

event = self.INPUT_EVENT % self._test_timestamp self.pub_client.publish(event.encode('utf-8'))

I would expect users to follow a similar pattern in their pipelines, and they might refer to beam examples for guidance, so I suggest to change this.

tvalentyn · 2019-03-20T16:29:33Z

sdks/python/apache_beam/examples/complete/game/game_stats_it_test.py


  # Input events containing user, team, score, processing time, window start.
-  INPUT_EVENT = 'user1,teamA,10,%d,2015-11-02 09:09:28.224'
+  INPUT_EVENT = b'user1,teamA,10,%d,2015-11-02 09:09:28.224'


I am not sure if this should be bytes. Does self.pub_client.publish require bytes? If so, we should encode before passing data to that method,

To quote https://docs.python.org/3/howto/pyporting.html , Decode binary data to text as soon as possible, encode text as binary data as late as possible

Juta · 2019-03-22T15:14:53Z

Run Python PostCommit

Juta · 2019-04-02T08:43:49Z

Run Python PostCommit

Juta · 2019-04-02T10:01:53Z

Run Python PostCommit

… postcommit

Juta · 2019-04-02T12:31:48Z

Run Python PostCommit

tvalentyn

Thanks, @Juta!

aaltay · 2019-04-02T17:09:15Z

Thank you @Juta and @tvalentyn

Juta force-pushed the bq-io-tests branch from 53eb74a to d64ff06 Compare March 19, 2019 13:25

Juta force-pushed the bq-io-tests branch 5 times, most recently from 49cbdeb to a12b64b Compare March 19, 2019 15:55

Juta force-pushed the bq-io-tests branch 2 times, most recently from fc57d0e to 98d7f0f Compare March 20, 2019 09:45

Juta force-pushed the bq-io-tests branch 2 times, most recently from 9a05dd2 to 9aa9f37 Compare March 20, 2019 14:12

tvalentyn requested changes Mar 20, 2019

View reviewed changes

Juta force-pushed the bq-io-tests branch from 9aa9f37 to 24e271c Compare March 22, 2019 15:13

tvalentyn mentioned this pull request Mar 27, 2019

[BEAM-6914] Reverting behavior of Native BQ sink in Python #8143

Merged

Juta force-pushed the bq-io-tests branch from 24e271c to d75d599 Compare April 2, 2019 08:41

Juta force-pushed the bq-io-tests branch 2 times, most recently from c11d669 to c548b69 Compare April 2, 2019 10:01

[BEAM-6619] [BEAM-6593] [BEAM-6711] Add bigquery integration tests to…

9dfd935

… postcommit

Juta force-pushed the bq-io-tests branch from c548b69 to 9dfd935 Compare April 2, 2019 12:31

Juta changed the title ~~[BEAM-6619] [BEAM-6593] Add bigquery integration tests to postcommit~~ [BEAM-6619] [BEAM-6593] Add example integration tests to postcommit Apr 2, 2019

tvalentyn approved these changes Apr 2, 2019

View reviewed changes

aaltay approved these changes Apr 2, 2019

View reviewed changes

aaltay merged commit b6800ce into apache:master Apr 2, 2019

[BEAM-6619] [BEAM-6593] Add example integration tests to postcommit #8076

[BEAM-6619] [BEAM-6593] Add example integration tests to postcommit #8076

Uh oh!

Conversation

Juta commented Mar 18, 2019

Post-Commit Tests Status (on master branch)

Uh oh!

Juta commented Mar 18, 2019

Uh oh!

Juta commented Mar 19, 2019

Uh oh!

Juta commented Mar 19, 2019

Uh oh!

Juta commented Mar 20, 2019

Uh oh!

Juta commented Mar 20, 2019

Uh oh!

Juta commented Mar 20, 2019

Uh oh!

tvalentyn 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!

Juta Mar 22, 2019 • 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!

Juta commented Mar 22, 2019

Uh oh!

Juta commented Apr 2, 2019

Uh oh!

Juta commented Apr 2, 2019

Uh oh!

Juta commented Apr 2, 2019

Uh oh!

tvalentyn left a comment

Choose a reason for hiding this comment

Uh oh!

aaltay commented Apr 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Juta Mar 22, 2019 •

edited

Loading