robinyqiu commented on a change in pull request #12266:
URL: https://github.com/apache/beam/pull/12266#discussion_r455492426
##########
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaCapableIOProvider.java
##########
@@ -264,4 +265,22 @@ private static boolean fieldPresent(
schema.getField(field).getType(),
Schema.EquivalenceNullablePolicy.IGNORE);
}
}
+
+ @Internal
+ @AutoValue
+ public abstract static class Config implements Serializable {
Review comment:
Can we make this class private or package-private?
##########
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaCapableIOProvider.java
##########
@@ -155,16 +159,17 @@ private void validateConfigurationSchema(Row
configuration) {
/** An abstraction to create schema aware IOs. */
@Internal
private static class PubsubSchemaIO implements SchemaIO, Serializable {
Review comment:
The `@Internal` flag is a warning to users to not depend on the labeled
class. This class is not even visible to user because it is private, so the
flag is not needed here.
##########
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaCapableIOProvider.java
##########
@@ -264,4 +265,22 @@ private static boolean fieldPresent(
schema.getField(field).getType(),
Schema.EquivalenceNullablePolicy.IGNORE);
}
}
+
+ @Internal
+ @AutoValue
+ public abstract static class Config implements Serializable {
+ @Nullable
+ public abstract String getTimestampAttributeKey();
+
+ @Nullable
+ public abstract String getDeadLetterQueue();
+
+ private boolean useDlqCheck() {
Review comment:
Rename to `useDeadLetterQueue` for consistency?
##########
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaCapableIOProvider.java
##########
@@ -211,7 +216,11 @@ public POutput expand(PCollection<Row> input) {
return input
.apply(
RowToPubsubMessage.fromConfig(
- config, useFlatSchema, useTimestampAttribute(config)))
+ new AutoValueSchema()
Review comment:
Actually, I looked at the `RowToPubsubMessage` class and found 2 of the
3 parameters here (`config` and `useFlatSchema`) are not used at all. What was
the reason to keep them there? Can we remove them?
----------------------------------------------------------------
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]
