robinyqiu commented on a change in pull request #12266:
URL: https://github.com/apache/beam/pull/12266#discussion_r456089799
##########
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/RowToPubsubMessage.java
##########
@@ -40,19 +39,14 @@
* whether config.getValue("timestampAttributeKey") is set.
*/
class RowToPubsubMessage extends PTransform<PCollection<Row>,
PCollection<PubsubMessage>> {
- private final Row config;
private final Boolean useTimestampAttribute;
- private RowToPubsubMessage(Row config, Boolean useFlatSchema, Boolean
useTimestampAttribute) {
- checkArgument(useFlatSchema, "RowToPubsubMessage is only supported for
flattened schemas.");
-
- this.config = config;
+ private RowToPubsubMessage(Boolean useTimestampAttribute) {
Review comment:
Java coding style discourage using boxed primitive types, unless there
are reasons. I think here it's better to just use `boolean`. Same below.
##########
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/RowToPubsubMessage.java
##########
@@ -40,19 +39,14 @@
* whether config.getValue("timestampAttributeKey") is set.
*/
class RowToPubsubMessage extends PTransform<PCollection<Row>,
PCollection<PubsubMessage>> {
- private final Row config;
private final Boolean useTimestampAttribute;
- private RowToPubsubMessage(Row config, Boolean useFlatSchema, Boolean
useTimestampAttribute) {
- checkArgument(useFlatSchema, "RowToPubsubMessage is only supported for
flattened schemas.");
-
- this.config = config;
+ private RowToPubsubMessage(Boolean useTimestampAttribute) {
this.useTimestampAttribute = useTimestampAttribute;
}
- public static RowToPubsubMessage fromConfig(
- Row config, Boolean useFlatSchema, Boolean useTimestampAttribute) {
- return new RowToPubsubMessage(config, useFlatSchema,
useTimestampAttribute);
+ public static RowToPubsubMessage fromConfig(Boolean useTimestampAttribute) {
Review comment:
Nit: Now that `config` parameter is gone, this class could be better
named as something more meaningful like `withTimestampAttribute()`.
----------------------------------------------------------------
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]
