gemelen commented on code in PR #22216:
URL: https://github.com/apache/beam/pull/22216#discussion_r918926330
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubIO.java:
##########
@@ -524,6 +524,16 @@ public static Read<PubsubMessage>
readMessagesWithAttributesAndMessageId() {
.build();
}
+ /**
+ * Returns A {@link PTransform} that continuously reads from a Google Cloud
Pub/Sub stream. The
+ * messages will contain a {@link PubsubMessage#getPayload() payload}, {@link
+ * PubsubMessage#getAttributeMap() attributes}, along with the {@link
PubsubMessage#getMessageId()
+ * messageId} and {PubsubMessage#getOrderingKey() orderingKey} from PubSub.
+ */
+ public static Read<PubsubMessage>
readMessagesWithAllAttributesAndMessageIdAndOrderingKey() {
Review Comment:
I think that since this coder grabs all fields of an incoming PubsubMessage
then it could be named without specifying all these fields in its name.
Accordingly, method would be better named `readMessages` but this name is
already taken by other implementation (that reads only payload of
PubsubMessage).
I agree that `readMessagesWithAllAttributesAndMessageIdAndOrderingKey` then
should be named `readMessagesWithAttributesAndMessageIdAndOrderingKey`, so I'm
updating it to this.
Regardint the
> Also, based on the above method, do we need .setNeedsAttributes(true) here?
newly added method doesn't include `setNeedsAttributes`, it's a
`setNeedsOrderingKey`:
```
public static Read<PubsubMessage>
readMessagesWithAllAttributesAndMessageIdAndOrderingKey()
{
return Read.newBuilder()
.setCoder(PubsubMessageCoder.of())
.setNeedsOrderingKey(true)
.build();
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]