ahmedabu98 commented on code in PR #32465:
URL: https://github.com/apache/beam/pull/32465#discussion_r1760874735
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubIO.java:
##########
@@ -1390,6 +1392,7 @@ static <T> Builder<T> newBuilder(
builder.setFormatFn(formatFn);
builder.setBadRecordRouter(BadRecordRouter.THROWING_ROUTER);
builder.setBadRecordErrorHandler(new DefaultErrorHandler<>());
+ builder.setValidate(true);
Review Comment:
This is the entry method for all writes, so it's enough to
`setValidate(true)` here and remove it from the other methods.
It's better to only set it here, otherwise validation will be set to true
even when someone doesn't want it. Take the following for example:
```java
PubsubIO
.writeStrings() // validate = true
.withoutValidation() // validate = false
.to("topic"); // validate = true
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubIO.java:
##########
@@ -872,6 +875,7 @@ static <T> Builder<T>
newBuilder(SerializableFunction<PubsubMessage, T> parseFn)
builder.setNeedsOrderingKey(false);
builder.setBadRecordRouter(BadRecordRouter.THROWING_ROUTER);
builder.setBadRecordErrorHandler(new DefaultErrorHandler<>());
+ builder.setValidate(true);
Review Comment:
Similar to the above comment, can we only `setValidate(true)` here and
remove it from the other methods?
##########
CHANGES.md:
##########
@@ -62,7 +62,7 @@
## I/Os
-* Support for X source added (Java/Python)
([#X](https://github.com/apache/beam/issues/X)).
+* Check Pub/Sub topic is existing before Read/Write (Java)
([#32465](https://github.com/apache/beam/pull/32465))
Review Comment:
```suggestion
* PubsubIO will validate that the Pub/Sub topic exists before running the
Read/Write pipeline (Java) ([#32465](https://github.com/apache/beam/pull/32465))
```
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubIOTest.java:
##########
@@ -997,4 +997,114 @@ public void testWithoutValidation() throws IOException {
read.validate(options);
}
+
+ @Test
+ public void testWriteTopicValidationSuccess() throws Exception {
+ PubsubIO.writeStrings().to("projects/my-project/topics/abc");
+ PubsubIO.writeStrings().to("projects/my-project/topics/ABC");
+ PubsubIO.writeStrings().to("projects/my-project/topics/AbC-DeF");
+ PubsubIO.writeStrings().to("projects/my-project/topics/AbC-1234");
+
PubsubIO.writeStrings().to("projects/my-project/topics/AbC-1234-_.~%+-_.~%+-_.~%+-abc");
+ PubsubIO.writeStrings()
+ .to(
+ new StringBuilder()
+ .append("projects/my-project/topics/A-really-long-one-")
+ .append(
+
"111111111111111111111111111111111111111111111111111111111111111111111111111111111")
Review Comment:
nit: for these long strings, can we use something like
`RandomStringUtils.randomAlphanumeric()` instead?
--
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]
