You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "damccorm (via GitHub)" <gi...@apache.org> on 2026/06/12 15:15:36 UTC
[PR] Improve BigQueryIO Storage Write API error messages [beam]
damccorm opened a new pull request, #38948:
URL: https://github.com/apache/beam/pull/38948
This PR improves the error messages thrown by the BigQuery Storage Write API (both sharded and unsharded paths) when retries are exhausted or when persistent errors are encountered.
### Changes
* **Surface Root Cause**: The `RuntimeException` thrown when `AppendRows` retries are exhausted now includes the last encountered error (root cause) in the exception message and propagates it as the cause of the `RuntimeException`.
* **Elevate Logging Level**: Elevated the final failure logging to `ERROR` level before throwing the exception, ensuring it is visible even if the log level is set to `ERROR`.
* **Actionable Advice**: Added actionable advice to the error message if the error is `PERMISSION_DENIED` or `NOT_FOUND`, suggesting the user check if the destination table exists and if the service account has the necessary `TABLES_UPDATE_DATA` permission.
* **Testing**: Added a new parameterized unit test `testStorageApiWriteFailureExhaustedRetries` to `BigQueryIOWriteTest.java` that simulates `AppendRows` failures (using new mock capabilities added to `FakeDatasetService`) and verifies the improved error messages.
These changes should significantly reduce troubleshooting time for users encountering write failures with the Storage Write API.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #38948:
URL: https://github.com/apache/beam/pull/38948#issuecomment-4693068501
/gemini review
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #38948:
URL: https://github.com/apache/beam/pull/38948#issuecomment-4694012147
R: @ahmedabu98
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "derrickaw (via GitHub)" <gi...@apache.org>.
derrickaw commented on PR #38948:
URL: https://github.com/apache/beam/pull/38948#issuecomment-4916223424
Hi @ahmedabu98, can you take a look at this PR at your convenience? Thanks!
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on PR #38948:
URL: https://github.com/apache/beam/pull/38948#issuecomment-4694147738
> [!WARNING]
> Gemini encountered an error creating the summary. You can try again by commenting `/gemini summary`.
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm merged PR #38948:
URL: https://github.com/apache/beam/pull/38948
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #38948:
URL: https://github.com/apache/beam/pull/38948#discussion_r3405463750
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiWritesShardedRecords.java:
##########
@@ -1066,7 +1066,25 @@ public void process(
if (numAppends > 0) {
initializeContexts.accept(contexts);
- retryManager.run(true);
+ try {
+ retryManager.run(true);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw e;
+ } catch (Exception e) {
+ Status.Code statusCode = Status.fromThrowable(e).getCode();
+ String errorMessage =
+ String.format(
+ "More than %d attempts to call AppendRows failed. Last encountered error: %s",
+ maxRetries, e.toString());
+ if (statusCode == Status.Code.PERMISSION_DENIED
+ || statusCode == Status.Code.NOT_FOUND) {
+ errorMessage +=
+ ". Please check if the destination table exists and if the service account has the "
+ + "bigquery.tables.updateData permission.";
+ }
Review Comment:
The error message formatting logic here is duplicated with `StorageApiWriteUnshardedRecords`. Consider reusing the extracted shared helper method `StorageApiWriteUnshardedRecords.buildAppendRowsErrorMessage` to simplify this catch block and avoid code duplication.
```java
String errorMessage =\n StorageApiWriteUnshardedRecords.buildAppendRowsErrorMessage(maxRetries, e);
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiWriteUnshardedRecords.java:
##########
@@ -792,9 +792,17 @@ 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 == Status.Code.PERMISSION_DENIED
+ || statusCode == Status.Code.NOT_FOUND) {
+ errorMessage +=
+ ". Please check if the destination table exists and if the service account has the "
+ + "bigquery.tables.updateData permission.";
+ }
Review Comment:
The error message formatting logic is duplicated between `StorageApiWriteUnshardedRecords` and `StorageApiWritesShardedRecords`. To improve maintainability and ensure consistency, consider extracting this logic into a shared package-private static helper method in `StorageApiWriteUnshardedRecords`.\n\nFor example, you can define the helper method in `StorageApiWriteUnshardedRecords`:\n\n```java\nstatic String buildAppendRowsErrorMessage(int allowedRetry, Throwable error) {\n io.grpc.Status.Code statusCode = io.grpc.Status.fromThrowable(error).getCode();\n String errorMessage =\n String.format(\n "More than %d attempts to call AppendRows failed. Last encountered error: %s",\n allowedRetry, error != null ? error.toString() : "unknown");\n if (statusCode == io.grpc.Status.Code.PERMISSION_DENIED\n || statusCode == io.grpc.Status.Code.NOT_FOUND) {\n errorMessage +=\n ". Please check if the destination table exists and if the service account has
the "\n + "bigquery.tables.updateData permission.";\n }\n return errorMessage;\n}\n```
```suggestion
String errorMessage = buildAppendRowsErrorMessage(allowedRetry, error);
```
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #38948:
URL: https://github.com/apache/beam/pull/38948#discussion_r3404728498
##########
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 == Status.Code.PERMISSION_DENIED
+ || statusCode == Status.Code.NOT_FOUND) {
+ errorMessage +=
+ ". Please check if the destination table exists and if the service account has the "
+ + "TABLES_UPDATE_DATA permission.";
+ }
+ LOG.error("{}", errorMessage, error);
+ throw new RuntimeException(errorMessage, error);
Review Comment:
Logging the exception immediately before throwing it is an anti-pattern. In Apache Beam, the runner will catch and log any thrown exception at the `ERROR` level, so logging it here results in duplicate stack traces in the worker logs, making troubleshooting more difficult. Removing the `LOG.error` call keeps the logs clean.
```suggestion
String errorMessage =
String.format(
"More than %d attempts to call AppendRows failed. Last encountered error: %s",
allowedRetry, error != null ? error.toString() : "unknown");
if (statusCode == Status.Code.PERMISSION_DENIED
|| statusCode == Status.Code.NOT_FOUND) {
errorMessage +=
". Please check if the destination table exists and if the service account has the "
+ "TABLES_UPDATE_DATA permission.";
}
throw new RuntimeException(errorMessage, error);
```
##########
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 == Status.Code.PERMISSION_DENIED
+ || statusCode == Status.Code.NOT_FOUND) {
+ errorMessage +=
+ ". Please check if the destination table exists and if the service account has the "
+ + "TABLES_UPDATE_DATA permission.";
+ }
+ LOG.error("{}", errorMessage, cause);
+ throw new RuntimeException(errorMessage, cause);
+ }
Review Comment:
There are a few issues here:
1. **Duplicate Logging**: Logging the exception right before throwing it causes duplicate stack traces in the runner logs.
2. **Lost Stack Trace**: Throwing `new RuntimeException(errorMessage, cause)` where `cause = e.getCause()` discards the stack trace of the outer exception `e`. It is better to pass `e` directly to preserve the full exception chain.
3. **Simplification**: `Status.fromThrowable` already traverses the cause chain internally, so you can pass `e` directly to it and avoid manual null-checking and extraction of `e.getCause()`.
```java
try {
retryManager.run(true);
} catch (Exception e) {
Status.Code statusCode = Status.fromThrowable(e).getCode();
String errorMessage =
String.format(
"More than %d attempts to call AppendRows failed. Last encountered error: %s",
maxRetries, e.toString());
if (statusCode == Status.Code.PERMISSION_DENIED
|| statusCode == Status.Code.NOT_FOUND) {
errorMessage +=
". Please check if the destination table exists and if the service account has the "
+ "TABLES_UPDATE_DATA permission.";
}
throw new RuntimeException(errorMessage, 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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #38948:
URL: https://github.com/apache/beam/pull/38948#issuecomment-4693352138
/gemini review
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on code in PR #38948:
URL: https://github.com/apache/beam/pull/38948#discussion_r3404992050
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiWritesShardedRecords.java:
##########
@@ -1066,7 +1066,22 @@ public void process(
if (numAppends > 0) {
initializeContexts.accept(contexts);
- retryManager.run(true);
+ try {
+ retryManager.run(true);
+ } catch (Exception e) {
+ Status.Code statusCode = Status.fromThrowable(e).getCode();
+ String errorMessage =
+ String.format(
+ "More than %d attempts to call AppendRows failed. Last encountered error: %s",
+ maxRetries, e.toString());
+ if (statusCode == Status.Code.PERMISSION_DENIED
+ || statusCode == Status.Code.NOT_FOUND) {
+ errorMessage +=
+ ". Please check if the destination table exists and if the service account has the "
+ + "TABLES_UPDATE_DATA permission.";
+ }
+ throw new RuntimeException(errorMessage, e);
+ }
Review Comment:
I don't think we need to do this - InterruptedException is not going to be a normal outcome here
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38948:
URL: https://github.com/apache/beam/pull/38948#issuecomment-4694021068
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment `assign set of reviewers`
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #38948:
URL: https://github.com/apache/beam/pull/38948#issuecomment-4693620396
/gemini review
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on PR #38948:
URL: https://github.com/apache/beam/pull/38948#issuecomment-4694099530
## Summary of Changes
Hello, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the debugging experience for users of the BigQuery Storage Write API by providing more descriptive and actionable error messages. When AppendRows operations fail after exhausting retries or encountering persistent issues, the system will now output detailed information, including the last error encountered and specific guidance on common problems like incorrect permissions or missing tables. This change aims to reduce the time and effort required to diagnose and resolve write failures.
### Highlights
* **Enhanced Error Messages**: Improved error messages for BigQuery Storage Write API failures, providing more context on the last encountered error and suggesting potential causes like missing permissions or non-existent tables.
* **Error Handling in Sharded Writes**: Implemented try-catch blocks in the sharded write path to capture and re-throw exceptions with the new, more informative error messages.
* **Testability Improvements**: Modified the FakeDatasetService to allow injecting AppendRows errors, enabling robust testing of error handling scenarios.
* **New Test Case**: Added a new test `testStorageApiWriteFailureExhaustedRetries` to validate the improved error messages when retries are exhausted due to permission issues.
<details>
<summary><b>New Features</b></summary>
🧠You can now enable **Memory** (public preview) to help **Gemini Code Assist** learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. **Click [here](https://codeassist.google/code-review/login) to enable Memory in your admin console.**
</details>
<details>
<summary><b>Using Gemini Code Assist</b></summary>
<br>
The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips.
<b>Invoking Gemini</b>
You can request assistance from Gemini at any point by creating a comment using either `/gemini <command>` or `@gemini-code-assist <command>`. Below is a summary of the supported commands on the current page.
Feature | Command | Description
--- | --- | ---
Code Review | `/gemini review` | Performs a code review for the current pull request in its current state.
Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state.
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help | `/gemini help` | Displays a list of available commands.
<b>Customization</b>
To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github).
<b>Limitations & Feedback</b>
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG).
</details>
[^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on PR #38948:
URL: https://github.com/apache/beam/pull/38948#issuecomment-4694110218
## Summary of Changes
Hello, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request improves the developer experience when working with the BigQuery Storage Write API by providing more actionable error messages. By capturing and surfacing the last encountered error and offering specific troubleshooting advice for common permission and configuration issues, it helps users diagnose and resolve failures more efficiently when retries are exhausted.
### Highlights
* **Enhanced Error Messaging**: Updated BigQueryIO Storage Write API to provide more descriptive error messages when retries are exhausted, including the last encountered error.
* **Actionable Permission Feedback**: Added specific guidance for PERMISSION_DENIED and NOT_FOUND errors, suggesting checks for table existence and service account permissions.
* **Improved Exception Handling**: Wrapped retry logic in sharded and unsharded paths to ensure failures are caught and reported with the new, detailed error context.
* **Test Coverage**: Introduced a new test case to verify that exhausted retries correctly trigger the updated error messages and suggestions.
<details>
<summary><b>New Features</b></summary>
🧠You can now enable **Memory** (public preview) to help **Gemini Code Assist** learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. **Click [here](https://codeassist.google/code-review/login) to enable Memory in your admin console.**
</details>
<details>
<summary><b>Using Gemini Code Assist</b></summary>
<br>
The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips.
<b>Invoking Gemini</b>
You can request assistance from Gemini at any point by creating a comment using either `/gemini <command>` or `@gemini-code-assist <command>`. Below is a summary of the supported commands on the current page.
Feature | Command | Description
--- | --- | ---
Code Review | `/gemini review` | Performs a code review for the current pull request in its current state.
Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state.
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help | `/gemini help` | Displays a list of available commands.
<b>Customization</b>
To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github).
<b>Limitations & Feedback</b>
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG).
</details>
[^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #38948:
URL: https://github.com/apache/beam/pull/38948#discussion_r3404920785
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiWritesShardedRecords.java:
##########
@@ -1066,7 +1066,22 @@ public void process(
if (numAppends > 0) {
initializeContexts.accept(contexts);
- retryManager.run(true);
+ try {
+ retryManager.run(true);
+ } catch (Exception e) {
+ Status.Code statusCode = Status.fromThrowable(e).getCode();
+ String errorMessage =
+ String.format(
+ "More than %d attempts to call AppendRows failed. Last encountered error: %s",
+ maxRetries, e.toString());
+ if (statusCode == Status.Code.PERMISSION_DENIED
+ || statusCode == Status.Code.NOT_FOUND) {
+ errorMessage +=
+ ". Please check if the destination table exists and if the service account has the "
+ + "TABLES_UPDATE_DATA permission.";
+ }
+ throw new RuntimeException(errorMessage, e);
+ }
Review Comment:
When catching `Exception` from `retryManager.run(true)`, we should handle `InterruptedException` separately. Wrapping `InterruptedException` in a `RuntimeException` without restoring the interrupted status is a Java anti-pattern that can prevent clean thread shutdown in multi-threaded environments. Additionally, the IAM permission name should be updated to `bigquery.tables.updateData` to match the official GCP permission.
```java
try {
retryManager.run(true);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw e;
} catch (Exception e) {
Status.Code statusCode = Status.fromThrowable(e).getCode();
String errorMessage =
String.format(
"More than %d attempts to call AppendRows failed. Last encountered error: %s",
maxRetries, e.toString());
if (statusCode == Status.Code.PERMISSION_DENIED
|| statusCode == Status.Code.NOT_FOUND) {
errorMessage +=
". Please check if the destination table exists and if the service account has the "
+ "bigquery.tables.updateData permission.";
}
throw new RuntimeException(errorMessage, e);
}
```
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIOWriteTest.java:
##########
@@ -1659,6 +1659,48 @@ public void testStreamingStorageApiWriteWithErrorHandling() throws Exception {
storageWriteWithErrorHandling(false);
}
+ @Test
+ public void testStorageApiWriteFailureExhaustedRetries() throws Exception {
+ assumeTrue(useStorageApi);
+
+ // Set up fake dataset service to return PERMISSION_DENIED for appendRows
+ fakeDatasetService.setAppendRowsError(
+ new io.grpc.StatusRuntimeException(
+ io.grpc.Status.PERMISSION_DENIED.withDescription("Missing permissions")));
+
+ List<Integer> elements = Lists.newArrayList(1, 2, 3);
+
+ BigQueryIO.Write<Integer> write =
+ BigQueryIO.<Integer>write()
+ .to("project-id:dataset-id.table-id")
+ .withCreateDisposition(BigQueryIO.Write.CreateDisposition.CREATE_NEVER)
+ .withFormatFunction(
+ (SerializableFunction<Integer, TableRow>)
+ input -> new TableRow().set("number", input))
+ .withSchema(
+ new TableSchema()
+ .setFields(
+ ImmutableList.of(
+ new TableFieldSchema().setName("number").setType("INTEGER"))))
+ .withTestServices(fakeBqServices)
+ .withoutValidation();
+
+ if (useStreaming) {
+ write = write.withTriggeringFrequency(Duration.standardSeconds(30));
+ }
+
+ PCollection<Integer> input = p.apply(Create.of(elements).withCoder(BigEndianIntegerCoder.of()));
+ input.apply("WriteToBQ", write);
+
+ thrown.expect(RuntimeException.class);
+ thrown.expectMessage("More than");
+ thrown.expectMessage("attempts to call AppendRows failed");
+ thrown.expectMessage("PERMISSION_DENIED");
+ thrown.expectMessage("TABLES_UPDATE_DATA");
Review Comment:
Update the expected error message in the test to match the updated permission name `bigquery.tables.updateData`.
```suggestion
thrown.expectMessage("bigquery.tables.updateData");
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiWriteUnshardedRecords.java:
##########
@@ -792,9 +792,17 @@ 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 == Status.Code.PERMISSION_DENIED
+ || statusCode == Status.Code.NOT_FOUND) {
+ errorMessage +=
+ ". Please check if the destination table exists and if the service account has the "
+ + "TABLES_UPDATE_DATA permission.";
+ }
Review Comment:
The IAM permission required to write data to a BigQuery table is `bigquery.tables.updateData`. Using the exact permission name instead of `TABLES_UPDATE_DATA` makes the error message much more helpful for users configuring IAM roles.
```suggestion
if (statusCode == Status.Code.PERMISSION_DENIED
|| statusCode == Status.Code.NOT_FOUND) {
errorMessage +=
". Please check if the destination table exists and if the service account has the "
+ "bigquery.tables.updateData permission.";
}
```
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "damccorm (via GitHub)" <gi...@apache.org>.
damccorm commented on PR #38948:
URL: https://github.com/apache/beam/pull/38948#issuecomment-4692805285
/gemini review
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve BigQueryIO Storage Write API error messages [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #38948:
URL: https://github.com/apache/beam/pull/38948#discussion_r3405453126
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIOWriteTest.java:
##########
@@ -1659,6 +1659,48 @@ public void testStreamingStorageApiWriteWithErrorHandling() throws Exception {
storageWriteWithErrorHandling(false);
}
+ @Test
+ public void testStorageApiWriteFailureExhaustedRetries() throws Exception {
+ assumeTrue(useStorageApi);
+
+ // Set up fake dataset service to return PERMISSION_DENIED for appendRows
+ fakeDatasetService.setAppendRowsError(
+ new io.grpc.StatusRuntimeException(
+ io.grpc.Status.PERMISSION_DENIED.withDescription("Missing permissions")));
+
+ List<Integer> elements = Lists.newArrayList(1, 2, 3);
+
+ BigQueryIO.Write<Integer> write =
+ BigQueryIO.<Integer>write()
+ .to("project-id:dataset-id.table-id")
+ .withCreateDisposition(BigQueryIO.Write.CreateDisposition.CREATE_NEVER)
+ .withFormatFunction(
+ (SerializableFunction<Integer, TableRow>)
+ input -> new TableRow().set("number", input))
+ .withSchema(
+ new TableSchema()
+ .setFields(
+ ImmutableList.of(
+ new TableFieldSchema().setName("number").setType("INTEGER"))))
+ .withTestServices(fakeBqServices)
+ .withoutValidation();
+
+ if (useStreaming) {
+ write = write.withTriggeringFrequency(Duration.standardSeconds(30));
+ }
+
+ PCollection<Integer> input = p.apply(Create.of(elements).withCoder(BigEndianIntegerCoder.of()));
+ input.apply("WriteToBQ", write);
+
+ thrown.expect(RuntimeException.class);
+ thrown.expectMessage("More than");
+ thrown.expectMessage("attempts to call AppendRows failed");
Review Comment:
Update the test assertions to match the simplified and more accurate error message from the sharded write path.
```suggestion
thrown.expectMessage("AppendRows failed");
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiWritesShardedRecords.java:
##########
@@ -1066,7 +1066,25 @@ public void process(
if (numAppends > 0) {
initializeContexts.accept(contexts);
- retryManager.run(true);
+ try {
+ retryManager.run(true);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw e;
+ } catch (Exception e) {
+ Status.Code statusCode = Status.fromThrowable(e).getCode();
+ String errorMessage =
+ String.format(
+ "More than %d attempts to call AppendRows failed. Last encountered error: %s",
+ maxRetries, e.toString());
Review Comment:
The error message states `"More than %d attempts to call AppendRows failed."` but this catch block catches any exception thrown by `retryManager.run(true)`. If a persistent/non-retryable error (such as `PERMISSION_DENIED` or `NOT_FOUND`) occurs on the very first attempt, the retry manager will fail immediately without retrying. In such cases, claiming that more than `%d` attempts failed is misleading. Consider using a more general message like `"AppendRows failed."` which is accurate for both single-attempt persistent failures and exhausted retries.
```suggestion
String errorMessage =
String.format(
"AppendRows failed. Last encountered error: %s",
e.toString());
```
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org