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

feat: add support to a few more specific StorageErrors for the Write API #1563

Merged
merged 11 commits into from
May 5, 2022
Merged

feat: add support to a few more specific StorageErrors for the Write API #1563

merged 11 commits into from
May 5, 2022

Conversation

stephaniewang526
Copy link
Contributor

OFFSET_OUT_OF_RANGE
OFFSET_ALREADY_EXISTS
STREAM_NOT_FOUND

Towards b/220198094

@stephaniewang526
feat: add support to a few more specific StorageErrors for the Write API
be4a20b 
OFFSET_OUT_OF_RANGE
OFFSET_ALREADY_EXISTS
STREAM_NOT_FOUND

Towards b/220198094
@stephaniewang526 stephaniewang526 requested review from a team and tswast March 4, 2022 16:15
@product-auto-label product-auto-label bot added the api: bigquerystorage Issues related to the googleapis/java-bigquerystorage API. label Mar 4, 2022
@stephaniewang526 stephaniewang526 added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 4, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 4, 2022
yirutang
yirutang reviewed Mar 4, 2022
View reviewed changes
Copy link
Contributor

@yirutang yirutang left a comment

Choose a reason for hiding this comment

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

Add a e2e test for all these exceptions + to show the StatusRuntimeException still works?

@stephaniewang526 stephaniewang526 marked this pull request as draft March 4, 2022 20:59
@stephaniewang526
Copy link
Contributor Author

Converting this to draft as we realized that there are backend changes needed (https://b.corp.google.com/issues/220198094#comment7) before proceeding.

@yirutang
Copy link
Contributor

The exception backend change is fully rolled out.

@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Apr 25, 2022
@stephaniewang526 stephaniewang526 marked this pull request as ready for review April 25, 2022 19:09
@stephaniewang526 stephaniewang526 requested a review from a team as a code owner April 25, 2022 20:30
@stephaniewang526 stephaniewang526 added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 25, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 25, 2022
yirutang
yirutang reviewed Apr 29, 2022
View reviewed changes
google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1/Exceptions.java
@@ -106,12 +141,32 @@ public static StorageException toStorageException(
if (error == null) {
return null;
}
String streamName = error.getEntity();
String errorMessage =
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment about this is to trim the entity sub message. Also narrow it further down by looking for "Entity: ".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Entity:..." is present in the detailMessage of OFFSET_OUT_OF_RANGE error when it should not be there since we already provide streamName in the error. The full detailMessage I get looks like: "OUT_OF_RANGE: The offset is within stream, expected offset 1, received 0 Entity: projects/.../datasets/.../tables/testtable/streams/CiQ2NGY3OT...".

I created b/231499869 to track this.

Copy link
Contributor

Choose a reason for hiding this comment

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

So why do you need to trim the next message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What does "next message" mean? I added a comment here to clarify further. Basically, I need to trim the message of whitespaces either way (with or without Entity) in order to get the "expected offset [0-9]+, received [0-9]+" portion to parse out the offset numbers.

google-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1/Exceptions.java Outdated
Long expectedOffet =
Long.parseLong(
errorMessage.substring(
errorMessage.lastIndexOf("offset") + 7, errorMessage.lastIndexOf(",")));
Copy link
Contributor

@yirutang yirutang Apr 29, 2022 •
edited

Choose a reason for hiding this comment

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

Have a stream match pattern for "expected offset $0, received $1". I am writing cl/445470232 to make sure the parsing will not break.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@stephaniewang526 stephaniewang526 added the owlbot:run Add this label to trigger the Owlbot post processor. label May 5, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 5, 2022
@stephaniewang526 stephaniewang526 added the owlbot:run Add this label to trigger the Owlbot post processor. label May 5, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label May 5, 2022
@stephaniewang526 stephaniewang526 merged commit c26091e into googleapis:main May 5, 2022
@release-please release-please bot mentioned this pull request May 5, 2022
Merged
gcf-merge-on-green bot pushed a commit that referenced this pull request May 5, 2022
@release-please
chore(main): release 2.13.0 (#1626)
4ac3a47 
🤖 I have created a release *beep* *boop*
---


## [2.13.0](v2.12.2...v2.13.0) (2022-05-05)


### Features

* add support to a few more specific StorageErrors for the Write API ([#1563](#1563)) ([c26091e](c26091e))
* next release from main branch is 2.12.2 ([#1624](#1624)) ([b2aa2a4](b2aa2a4))


### Bug Fixes

* A stuck when the client fail to get DoneCallback ([#1637](#1637)) ([3baa84e](3baa84e))
* Fix a possible NULL PTR after introduced timeout on waitForDone ([#1638](#1638)) ([e1c6ded](e1c6ded))


### Dependencies

* update dependency com.google.cloud:google-cloud-bigquery to v2.10.10 ([#1623](#1623)) ([54b74b8](54b74b8))
* update dependency org.apache.avro:avro to v1.11.0 ([#1632](#1632)) ([b47eea0](b47eea0))


### Documentation

* **samples:** update WriteComittedStream sample code to match best practices ([#1628](#1628)) ([5d4c7e1](5d4c7e1))
* **sample:** update WriteToDefaultStream sample to match best practices ([#1631](#1631)) ([73ddd7b](73ddd7b))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yirutang yirutang yirutang approved these changes

@tswast tswast Awaiting requested review from tswast

Assignees
No one assigned
Labels
api: bigquerystorage Issues related to the googleapis/java-bigquerystorage API. size: m Pull request size is medium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@stephaniewang526 @yirutang