You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2023/01/04 22:18:48 UTC
[GitHub] [beam] egalpin opened a new pull request, #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
egalpin opened a new pull request, #24887:
URL: https://github.com/apache/beam/pull/24887
This PR alters the default Coder for pubsub message objects from `PubsubMessageWithAttributesCoder` to `PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all `PubsubMessage.class`. Note that there was a previous dev@ thread[1] on this subject where there was no opposition to this change.
The one important detail to call out is that pipeline authors may need to explicitly set the coder to the value which was previously the default, `PubsubMessageWithAttributesCoder`, in cases where pipeline update is required.
Fixes #23525
[1] https://lists.apache.org/thread/c3qk0cp3rbhk2wh8m0z8gqxqy931dbwc
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/get-started-contributing/#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] apilloud commented on a diff in pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by GitBox <gi...@apache.org>.
apilloud commented on code in PR #24887:
URL: https://github.com/apache/beam/pull/24887#discussion_r1061964917
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder.java:
##########
@@ -42,8 +42,9 @@
// A message's attributes can be null.
private static final Coder<Map<String, String>> ATTRIBUTES_CODER =
NullableCoder.of(MapCoder.of(StringUtf8Coder.of(), StringUtf8Coder.of()));
- // A message's messageId cannot be null
- private static final Coder<String> MESSAGE_ID_CODER = StringUtf8Coder.of();
+ // A message's messageId can only be null when the message is an outgoing message (i.e. to be
+ // published). Incoming messages will always have a non-null messageId
+ private static final Coder<String> MESSAGE_ID_CODER = NullableCoder.of(StringUtf8Coder.of());
Review Comment:
This is a breaking change as it adds a byte to the encoded message. Why is it needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] egalpin commented on pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on PR #24887:
URL: https://github.com/apache/beam/pull/24887#issuecomment-1526870609
@kennknowles thanks for the bump. I have not had much bandwidth to meaningfully test this change. Something that concerns me and requires validation is whether or not the coder can be successfully set/changed from default so as to allow backward compatibility for pipeline authors. It appears that using .setCoder was not entirely sufficient based on prior exploration[1], but I have not yet revisited or gotten into a good repro-test feedback loop.
I have no issue handing this off if it’s high priority and someone else is keen to take up the work.
[1] https://github.com/apache/beam/issues/21162#issuecomment-1371259467
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24887:
URL: https://github.com/apache/beam/pull/24887#issuecomment-1371546138
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] egalpin commented on a diff in pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on code in PR #24887:
URL: https://github.com/apache/beam/pull/24887#discussion_r1179860391
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder.java:
##########
@@ -42,8 +42,9 @@
// A message's attributes can be null.
private static final Coder<Map<String, String>> ATTRIBUTES_CODER =
NullableCoder.of(MapCoder.of(StringUtf8Coder.of(), StringUtf8Coder.of()));
- // A message's messageId cannot be null
- private static final Coder<String> MESSAGE_ID_CODER = StringUtf8Coder.of();
+ // A message's messageId can only be null when the message is an outgoing message (i.e. to be
+ // published). Incoming messages will always have a non-null messageId
+ private static final Coder<String> MESSAGE_ID_CODER = NullableCoder.of(StringUtf8Coder.of());
Review Comment:
This would be needed to allow for use of this coder for outgoing messages, where the messageId field must be null to allow publishing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #24887:
URL: https://github.com/apache/beam/pull/24887#issuecomment-1609430974
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] closed pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
URL: https://github.com/apache/beam/pull/24887
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damccorm commented on pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #24887:
URL: https://github.com/apache/beam/pull/24887#issuecomment-1434784640
@egalpin and @apilloud should this be blocking the 2.46 release? I'm guessing no given that it wasn't a 2.45 blocker
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] egalpin commented on pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on PR #24887:
URL: https://github.com/apache/beam/pull/24887#issuecomment-1434848566
@damccorm i don’t believe it should be a blocker, no
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] egalpin commented on pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by "egalpin (via GitHub)" <gi...@apache.org>.
egalpin commented on PR #24887:
URL: https://github.com/apache/beam/pull/24887#issuecomment-1400616626
@johnjcasey no I don’t believe so
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] kennknowles commented on pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by "kennknowles (via GitHub)" <gi...@apache.org>.
kennknowles commented on PR #24887:
URL: https://github.com/apache/beam/pull/24887#issuecomment-1526086117
I notice this is attached to an open P1 for a while. What do we need to make progress?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] johnjcasey commented on pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by "johnjcasey (via GitHub)" <gi...@apache.org>.
johnjcasey commented on PR #24887:
URL: https://github.com/apache/beam/pull/24887#issuecomment-1400504807
@egalpin and @apilloud should this be blocking the 2.45 release?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] github-actions[bot] commented on pull request #24887: Sets PubsubMessageWithAttributesAndMessageIdAndOrderingKeyCoder as default coder for all PubsubMessage
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #24887:
URL: https://github.com/apache/beam/pull/24887#issuecomment-1620184540
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org