pcostell commented on code in PR #22966:
URL: https://github.com/apache/beam/pull/22966#discussion_r964230851
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/firestore/FirestoreV1ReadFn.java:
##########
@@ -484,6 +575,7 @@ public final void processElement(ProcessContext c) throws
Exception {
try {
RequestT request = nextPageToken == null ? element :
setPageToken(element, nextPageToken);
Review Comment:
It seems like the pattern for other setX(RequestT, X) is not not support
nullable and conditionally call it in the base. Perhaps we should do that as
well?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/firestore/FirestoreV1ReadFn.java:
##########
@@ -125,6 +134,13 @@ protected RunQueryRequest setStartFrom(
return element.toBuilder().setStructuredQuery(builder.build()).build();
}
+ @Override
+ protected RunQueryRequest setReadTime(RunQueryRequest element, @Nullable
Instant readTime) {
+ return readTime == null
+ ? element
Review Comment:
nit: We mix and match element / request for the variable names, can we
choose just one?
--
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]
