Hi Matthew, This looks like a reasonable approach. There is a difference how direct runner reads from pubsub compared to other runners. As you convert to a PR, please pay attention to the difference and add tests for both cases.
On Mon, Jul 29, 2019 at 8:35 AM Matthew Darwin < [email protected]> wrote: > Hi All, > > > This is my first attempt at a change for Beam on > https://issues.apache.org/jira/browse/BEAM-7819. This parses the > message_id when reading from the PubSub protobuf and adds to the > message_id, as suggested by the existing documentation - > https://beam.apache.org/releases/pydoc/2.13.0/apache_beam.io.gcp.pubsub.html#apache_beam.io.gcp.pubsub.PubsubMessage.attributes > > > My code is currently checked into my fork:- > https://github.com/matt-darwin/beam/tree/BEAM-7819-parse-pubsub-message_id > > > Prior to creating a pull request, I just wanted to check the approach is > sensible and then to go through the process for pull request to ensure it > goes smoothly. I do wonder if long term despite the documentation > indicated this would be part of the attributes property it might be more > sensible to fully mimic the protobuf for the PubSub message. > I agree that making it similar to the protobuf sounds like a more sensible approach. Take a look at how it is handled in Java. There is also value in consistency across languages. Final decision could be made in the PR. Ahmet /cc +Udi Meiri <[email protected]> > > > Kind regards > > Matthew >
