gemini-code-assist[bot] commented on code in PR #38948:
URL: https://github.com/apache/beam/pull/38948#discussion_r3404550324
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiWriteUnshardedRecords.java:
##########
@@ -792,9 +792,18 @@ long flush(
// Maximum number of times we retry before we fail the work item.
if (failedContext.failureCount > allowedRetry) {
- throw new RuntimeException(
+ String errorMessage =
String.format(
- "More than %d attempts to call AppendRows failed.",
allowedRetry));
+ "More than %d attempts to call AppendRows failed. Last
encountered error: %s",
+ allowedRetry, error != null ? error.toString() :
"unknown");
+ if (statusCode.equals(Status.Code.PERMISSION_DENIED)
+ || statusCode.equals(Status.Code.NOT_FOUND)) {
Review Comment:
Using `.equals()` on `statusCode` can throw a `NullPointerException` if
`statusCode` is null (which is possible if the underlying `error` is null or
unresolved). In Java, it is standard practice and safer to compare enums using
the `==` operator, which is inherently null-safe.
```suggestion
if (statusCode == Status.Code.PERMISSION_DENIED
|| statusCode == Status.Code.NOT_FOUND) {
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiWritesShardedRecords.java:
##########
@@ -1066,7 +1066,27 @@ public void process(
if (numAppends > 0) {
initializeContexts.accept(contexts);
- retryManager.run(true);
+ try {
+ retryManager.run(true);
+ } catch (Exception e) {
+ Throwable cause = e.getCause();
+ if (cause == null) {
+ cause = e;
+ }
+ Status.Code statusCode = Status.fromThrowable(cause).getCode();
+ String errorMessage =
+ String.format(
+ "More than %d attempts to call AppendRows failed. Last
encountered error: %s",
+ maxRetries, cause.toString());
+ if (statusCode.equals(Status.Code.PERMISSION_DENIED)
+ || statusCode.equals(Status.Code.NOT_FOUND)) {
Review Comment:
Using `.equals()` on `statusCode` can throw a `NullPointerException` if
`statusCode` is null. In Java, it is standard practice and safer to compare
enums using the `==` operator, which is inherently null-safe.
```suggestion
if (statusCode == Status.Code.PERMISSION_DENIED
|| statusCode == Status.Code.NOT_FOUND) {
```
--
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]
