gemini-code-assist[bot] commented on code in PR #38009:
URL: https://github.com/apache/beam/pull/38009#discussion_r3014480532
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageSourceBase.java:
##########
@@ -203,9 +205,54 @@ public List<BigQueryStorageStreamSource<T>> split(
@Override
public BoundedReader<T> createReader(PipelineOptions options) throws
IOException {
+ try {
+ if (split(0, options).isEmpty()) {
+ return new EmptyReader<>(this);
+ }
+ } catch (Exception e) {
+ // If split fails, we can't be sure if it's empty or not.
+ // For backwards compatibility with tests that don't mock everything,
+ // we still throw UnsupportedOperationException.
+ }
Review Comment:
Swallowing all exceptions here can obscure the actual cause of a failure
(such as authentication errors, permission issues, or API quota limits) when
`createReader` is invoked. While the comment explains this is for backward
compatibility with tests that may not have full mocking, it is highly
recommended to log the exception at a `DEBUG` level to facilitate
troubleshooting in production environments without being overly noisy.
```suggestion
} catch (Exception e) {
// If split fails, we can't be sure if it's empty or not.
// For backwards compatibility with tests that don't mock everything,
// we still throw UnsupportedOperationException.
LOG.debug("Split failed during createReader emptiness check", e);
}
```
--
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]
