sarthakbhutani commented on code in PR #34245:
URL: https://github.com/apache/beam/pull/34245#discussion_r2057750624
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigtable/BigtableServiceImpl.java:
##########
@@ -174,10 +177,18 @@ public boolean start() throws IOException {
query.filter(Filters.FILTERS.fromProto(rowFilter));
}
try {
- stream =
- client
- .readRowsCallable(new BigtableRowProtoAdapter())
- .call(query, GrpcCallContext.createDefault());
+ if (bigtableReadOptions != null
+ &&
Boolean.TRUE.equals(bigtableReadOptions.getExperimentalSkipLargeRows())) {
+ stream =
+ client
+ .skipLargeRowsCallable(new BigtableRowProtoAdapter())
+ .call(query, GrpcCallContext.createDefault());
+ } else {
+ stream =
+ client
+ .readRowsCallable(new BigtableRowProtoAdapter())
+ .call(query, GrpcCallContext.createDefault());
+ }
Review Comment:
1. regarding code suggestion -
instead of having if-else,
do we want to override the code later?
2. Resolving of the feature should be done during pipeline construction not
during execution
initially, i had made this into a separate reader class itself.
since this was a experimental option, It came out in the discussion that we
dont want the overhead of maintaining this as a separate reader for new changes.
and I had to revert these changes -
https://github.com/apache/beam/pull/34245/commits/50f792409a02bd31d37d4296d695f305647b91a1
--
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: github-unsubscr...@beam.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
