TheNeuralBit commented on a change in pull request #12297:
URL: https://github.com/apache/beam/pull/12297#discussion_r460341068
##########
File path:
sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/KinesisIO.java
##########
@@ -305,6 +306,10 @@ public static Read read() {
.build();
}
+ public static ReadData readData(Read read) {
+ return new ReadData(read);
+ }
+
Review comment:
I think this is an atypical API for an IO. If we add something like
`readData` to `KinesisIO` (instead of just hiding it in the
ExternalTransformBuilder) it would be preferable to make Read generic and have
two methods like this:
```java
public static Read<KinesisRecord> read() {
return new Read<KinesisRecord>();
}
public static Read<byte[]> readData() {
return new Read<byte[]>((record) -> record.getDataAsBytes());
}
```
That way they can both be configured in the same way:
```
PCollection<KinesisRecord> records =
p.apply(KinesisIO.read().withStreamName("streamName").with...)
PCollection<byte[]> data =
p.apply(KinesisIO.readData().withStreamName("streamName").with...)
```
This is what PubsubIO does:
https://github.com/apache/beam/blob/ecedd3e654352f1b51ab2caae0fd4665403bd0eb/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubIO.java#L439-L444
https://github.com/apache/beam/blob/ecedd3e654352f1b51ab2caae0fd4665403bd0eb/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubIO.java#L464-L469
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
