Skip to content
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.

Sign up for GitHub

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

Jump to bottom

docs(samples): Check for error from BatchCommitWriteStreams #940

Merged
merged 3 commits into from Mar 25, 2021
Merged

docs(samples): Check for error from BatchCommitWriteStreams #940

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
80c4870
docs(samples): Check for error from BatchCommitWriteStreams
VeronicaWasson Mar 15, 2021
87a7afb
Print error message from StorageError lists
VeronicaWasson Mar 22, 2021
181a2ba
fix formatting
VeronicaWasson Mar 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions samples/snippets/src/main/java/com/example/bigquerystorage/WritePendingStream.java
View file
Open in desktop
Expand Up @@ -90,6 +90,10 @@ public static void writePendingStream(String projectId, String datasetName, Stri
.build();
BatchCommitWriteStreamsResponse commitResponse =
client.batchCommitWriteStreams(commitRequest);
// If the response does not have a commit time, it means the commit operation failed.
if (commitResponse.hasCommitTime() == false) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it not return an error object in this case? User is expected to guess that something went wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user can call getStreamErrorsList to get a list of StorageError's. You could use this to look at the error per stream. Does it make sense to show this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense to just show the error objects rather than show hasCommitTime indirectly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per @yirutang checking for hasCommitTime is more semantically meaningful, because that's the error condition. I added code to display the list of error messages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. SGTM.

throw new RuntimeException("Error committing the streams");
}
System.out.println("Appended and committed records successfully.");
} catch (ExecutionException e) {
// If the wrapped exception is a StatusRuntimeException, check the state of the operation.
Expand Down