-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Add table information to BigQueryStorageApiInsertError #36832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add table information to BigQueryStorageApiInsertError #36832
Conversation
This change adds table identification (project, dataset, table) to
BigQueryStorageApiInsertError to help users identify which table
failed during Storage Write API operations.
Changes:
- Add tableUrn field to BigQueryStorageApiInsertError
- Add getProjectId(), getDatasetId(), getTableId() convenience methods
that parse the tableUrn on first access (lazy initialization with caching)
- Update StorageApiWriteUnshardedRecords to pass tableUrn to error objects
- Update StorageApiWritesShardedRecords to pass tableUrn to error objects
- Update StorageApiConvertMessages to pass tableUrn to error objects
- Update BigQueryStorageApiInsertErrorCoder to serialize/deserialize tableUrn
This makes the API consistent with BigQueryInsertError (used by
STREAMING_INSERTS method), which provides table information via
TableReference.
The tableUrn uses the format:
"projects/{project}/datasets/{dataset}/tables/{table}"
which is the standard format returned by TableDestination.getTableUrn().
Tested:
- All StorageApi-related tests pass
- Verified table information is correctly captured in error outputs
Summary of ChangesHello @hekk-kaori-maeda, I'm Gemini Code Assist1! 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 improves the debugging and error handling capabilities for the BigQuery Storage Write API by embedding table identification directly into Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Assigning reviewers: R: @m-trieu for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
| } | ||
|
|
||
| public BigQueryStorageApiInsertError( | ||
| TableRow row, @Nullable String errorMessage, @Nullable String tableUrn) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe better to pass com. google. api. services. bigquery. model TableReference?
|
|
||
| @Nullable | ||
| public String getProjectId() { | ||
| return getParsedPart(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once you have tableReference, parsing is not needed.
| } | ||
|
|
||
| @Nullable | ||
| private String getParsedPart(int index) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use build in features/helpers eg BigQueryHelpers.parseTableSpec(tableSpec)
| throws IOException { | ||
| TABLE_ROW_CODER.encode(value.getRow(), outStream); | ||
| STRING_CODER.encode(value.getErrorMessage(), outStream); | ||
| STRING_CODER.encode(value.getTableUrn(), outStream); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use BigQueryHelpers.toTableSpec(tableReference)
| TABLE_ROW_CODER.decode(inStream), STRING_CODER.decode(inStream)); | ||
| TABLE_ROW_CODER.decode(inStream), | ||
| STRING_CODER.decode(inStream), | ||
| STRING_CODER.decode(inStream)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use BigQueryHelpers.parseTableSpec(STRING_CODER.decode(inStream))
| .output( | ||
| new BigQueryStorageApiInsertError( | ||
| failsafeTableRow, conversionException.toString())); | ||
| failsafeTableRow, conversionException.toString(), tableUrn)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tableDestination.getTableReference()
|
@hekk-kaori-maeda I really like PR! To be more consistent with BigQueryInsertError please use TableReference as field and expose it. |
…ueryStorageApiInsertError Following reviewer feedback, this change updates BigQueryStorageApiInsertError to use TableReference instead of String tableUrn for better consistency with BigQueryInsertError. Changes: - Changed BigQueryStorageApiInsertError to use TableReference field instead of String tableUrn - Updated BigQueryStorageApiInsertErrorCoder to use BigQueryHelpers.toTableSpec() and parseTableSpec() - Added null safety checks in coder to prevent NullPointerException when table is unknown - Updated all calling sites to pass TableReference from TableDestination.getTableReference(): - StorageApiWriteUnshardedRecords.java (3 locations) - StorageApiWritesShardedRecords.java (3 locations + added tableReference variable) - StorageApiConvertMessages.java (1 location) - Added TableReference imports to modified files The null checks in the coder ensure job stability even when table information is unavailable during error handling, preventing pipeline failures in error reporting scenarios.
Added the table field to the toString() method for better debugging and logging visibility. This ensures all fields are represented in the string output, making error investigation easier. Change: - Updated toString() to include ", table=" + table
|
@stankiewicz Changes made:
Ready for review. |
|
Reminder, please take a look at this pr: @m-trieu |
|
Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment R: @ahmedabu98 for label java. Available commands:
|
What is the purpose of the change?
This change adds table identification capabilities to
BigQueryStorageApiInsertErrorto help users identify and troubleshoot errors when using BigQuery Storage Write API,especially in pipelines that write to multiple tables.
Fixes #36831
What changes are included in this PR?
Added
tableUrnfield toBigQueryStorageApiInsertErrorprojects/{project}/datasets/{dataset}/tables/{table}TableDestination.getTableUrn()formatAdded convenience methods with lazy initialization
getProjectId(),getDatasetId(),getTableId()Updated
BigQueryStorageApiInsertErrorCodertableUrnfieldUpdated all calling sites to pass
tableUrnStorageApiWriteUnshardedRecords.java(3 locations)StorageApiWritesShardedRecords.java(3 locations)StorageApiConvertMessages.java(2 locations)This makes the API consistent with
BigQueryInsertError(used by STREAMING_INSERTS method).How was this change tested?
./gradlew :sdks:java:io:google-cloud-platform:test --tests "*StorageApi*"(PASSED)./gradlew :sdks:java:io:google-cloud-platform:spotlessApply(PASSED)Was this change documented?
Checklist
Fixes #36831CHANGES.md(will add if required by reviewers)