[BEAM-7819] Add missing fields to python apache_beam.io.gcp.pubsub.PubsubMessage by n-oden · Pull Request #14767 · apache/beam

n-oden · 2021-05-08T21:17:23Z

Add the publish_time, message_id and ordering_key
attributes to the PubsubMessage class
Convert google.protobuf.timestamp_pb2.Timestamp to python
datetime in _from_proto_str
Ensure message_id and publish_time fields are unset
in WriteToPubsub

This also addresses [BEAM-10958]

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
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.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

`ValidatesRunner` compliance status (on master branch)

Lang	ULR	Samza	Twister2
Go	---	---	---
Java
Python	---	---	---
XLang		---	---

Examples testing status on various runners

Lang	ULR	Dataflow	Flink	Samza	Spark	Twister2
Go	---	---	---	---	---	---	---
Java	---		---	---	---	---	---
Python	---	---	---	---	---	---	---
XLang	---	---	---	---	---	---	---

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Go	Java	Python

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
Non-portable
Portable	---			---	---	---

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

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

n-oden · 2021-05-08T21:29:09Z

R: @pabloem

Is it expected that the formatter/lint tests will fail? It's mostly indentation errors on lines I haven't changed

codecov · 2021-05-08T21:37:34Z

Codecov Report

Merging #14767 (591e2a1) into master (9dbc7b7) will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #14767      +/-   ##
==========================================
  Coverage   83.79%   83.79%              
==========================================
  Files         439      878     +439     
  Lines       59118   118266   +59148     
==========================================
+ Hits        49536    99101   +49565     
- Misses       9582    19165    +9583

Impacted Files	Coverage Δ
...sdks/python/apache_beam/portability/python_urns.py
...d/srcs/sdks/python/apache_beam/io/filebasedsink.py
..._beam/testing/benchmarks/nexmark/queries/query0.py
...ython/apache_beam/io/gcp/tests/bigquery_matcher.py
..._beam/testing/benchmarks/nexmark/queries/query5.py
...sdks/python/apache_beam/ml/gcp/visionml_test_it.py
...sdks/python/apache_beam/transforms/environments.py
..._beam/runners/portability/sdk_container_builder.py
...mples/snippets/transforms/elementwise/partition.py
...ache_beam/runners/portability/expansion_service.py
... and 1307 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9dbc7b7...591e2a1. Read the comment docs.

aaltay · 2021-05-09T19:25:04Z

R: @pabloem

Is it expected that the formatter/lint tests will fail? It's mostly indentation errors on lines I haven't changed

It seems to be failing on the lines you have changed. Please try fixing those.

n-oden · 2021-05-10T16:18:38Z

@aaltay ugh, my editor was configured to auto-apply Black formatting on save of any .py file and much ugliness ensued as a result. I've pried it all out and the format/lint tests are passing now.

...but obviously the precommit test is not. But the precommit test suite is so enormous that I'm having trouble seeing in the logs what portion of that might be related to my change. FWIW, python setup.py test -s apache_beam.io.gcp passes for me locally:

----------------------------------------------------------------------
Ran 365 tests in 38.591s

OK (skipped=38)

(Edit: spot-checked some of the failures, and it looks like apache_beam.runners.portability.flink_runner_test.FlinkRunnerTestStreaming is broken on master as well as in my branch so I'm going to tentatively assert that those failures are unrelated to this change)

aaltay · 2021-05-10T18:02:26Z

@aaltay ugh, my editor was configured to auto-apply Black formatting on save of any .py file and much ugliness ensued as a result. I've pried it all out and the format/lint tests are passing now.

...but obviously the precommit test is not. But the precommit test suite is so enormous that I'm having trouble seeing in the logs what portion of that might be related to my change. FWIW, python setup.py test -s apache_beam.io.gcp passes for me locally:
----------------------------------------------------------------------
Ran 365 tests in 38.591s

OK (skipped=38)
(Edit: spot-checked some of the failures, and it looks like apache_beam.runners.portability.flink_runner_test.FlinkRunnerTestStreaming is broken on master as well as in my branch so I'm going to tentatively assert that those failures are unrelated to this change)

If it is broken on master you do not need to worry about it now. Let's get it reviewed and hopefully master would be fixed by that time.

ibzib · 2021-05-10T18:07:21Z

(Edit: spot-checked some of the failures, and it looks like apache_beam.runners.portability.flink_runner_test.FlinkRunnerTestStreaming is broken on master as well as in my branch so I'm going to tentatively assert that those failures are unrelated to this change)

I don't see that FlinkRunnerTestStreaming failed in https://ci-beam.apache.org/job/beam_PreCommit_Python_Commit/18572/ (IIRC Flink tests are skipped in this job, so I wonder if you are testing a different command than the precommit uses?). Instead I see FnApiRunnerTestWithGrpcAndMultiWorkers.test_sdf_default_truncate_when_unbounded failed, which could be a flake. Re-running to confirm.

ibzib · 2021-05-10T18:07:39Z

Run Python PreCommit

ibzib · 2021-05-10T22:40:30Z

Run Python PreCommit

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

ibzib · 2021-05-10T23:30:32Z

Python precommit has finally passed, looks like the failures were indeed flakes. I filed BEAM-12322 to track this issue.

…bsubMessage - Add the `publish_time`, `message_id` and `ordering_key` attributes to the PubsubMessage class - Convert google.protobuf.timestamp_pb2.Timestamp to python datetime in _from_proto_str - Ensure `message_id` and `publish_time` fields are unset in WriteToPubsub

pabloem · 2021-05-12T16:48:34Z

Run Python PreCommit

pabloem · 2021-05-12T18:31:01Z

Run Python PreCommit

pabloem · 2021-05-12T18:31:08Z

Run Python 3.8 PostCommit

pabloem · 2021-05-12T22:27:54Z

LGTM. Thanks!

n-oden · 2021-06-17T18:56:21Z

@pabloem I was a little surprised to find that this didn't make it into the 2.30.0 release. Should I expect it in 2.31?

n-oden · 2021-06-18T18:24:07Z

(cc @aaltay and @ibzib -- is there a timeline for this actually landing on a release branch?)

ibzib · 2021-06-18T18:30:22Z

Here is a link to the release policy: https://beam.apache.org/contribute/release-guide/

Once the release branch is cut, you can check to see if your change is included. This change will be included in the 2.31.0 release.

ibzib · 2021-06-18T18:31:19Z

Release branches are publicly visible at release-2.x.0 e.g. https://github.com/apache/beam/commits/release-2.31.0?after=9161c9417a6c58cc54367e58370a64f8050a3e59+139&branch=release-2.31.0

astorrs · 2021-11-12T06:59:10Z

It’s been months now and this still hasn’t been included in a release. As @n-oden said back in June, “is there a timeline for this actually landing on a release branch?”

ibzib · 2021-11-12T16:44:51Z

@astorrs All changes are automatically picked up by a "release cut" that happens roughly once every 6 weeks. https://beam.apache.org/contribute/#when-will-my-change-show-up-in-an-apache-beam-release

This change was included starting with the 2.31.0 release of Beam in June. If you still see this bug in newer versions of Beam, please let us know.

aaltay requested a review from pabloem May 10, 2021 18:02

pabloem reviewed May 10, 2021

View reviewed changes

sdks/python/apache_beam/io/gcp/pubsub.py Outdated Show resolved Hide resolved

n-oden requested a review from pabloem May 12, 2021 14:43

pabloem merged commit c34aeae into apache:master May 12, 2021

n-oden deleted the add-missing-pubsubmessage-attrs branch May 13, 2021 15:23

damccorm mentioned this pull request Jun 4, 2022

Add getOrderingKey in org.apache.beam.sdk.io.gcp.pubsub.PubsubMessage #21304

Closed

Conversation

n-oden commented May 8, 2021

ValidatesRunner compliance status (on master branch)

Examples testing status on various runners

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

Uh oh!

n-oden commented May 8, 2021

Uh oh!

codecov bot commented May 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aaltay commented May 9, 2021

Uh oh!

n-oden commented May 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaltay commented May 10, 2021

Uh oh!

ibzib commented May 10, 2021

Uh oh!

ibzib commented May 10, 2021

Uh oh!

ibzib commented May 10, 2021

Uh oh!

Uh oh!

ibzib commented May 10, 2021

Uh oh!

pabloem commented May 12, 2021

Uh oh!

pabloem commented May 12, 2021

Uh oh!

pabloem commented May 12, 2021

Uh oh!

pabloem commented May 12, 2021

Uh oh!

n-oden commented Jun 17, 2021

Uh oh!

n-oden commented Jun 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ibzib commented Jun 18, 2021

Uh oh!

ibzib commented Jun 18, 2021

Uh oh!

astorrs commented Nov 12, 2021

Uh oh!

ibzib commented Nov 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

`ValidatesRunner` compliance status (on master branch)

codecov bot commented May 8, 2021 •

edited

Loading

n-oden commented May 10, 2021 •

edited

Loading

n-oden commented Jun 18, 2021 •

edited

Loading