FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: update grpc bidi resumable uploads to validate ack'd object size by BenWhitehead · Pull Request #2570 · googleapis/java-storage · GitHub

fix: update grpc bidi resumable uploads to validate ack'd object size - #2570

Merged
BenWhitehead merged 6 commits into
mainfrom
fix/grpc-bidi-validation
Jun 13, 2024
Merged

fix: update grpc bidi resumable uploads to validate ack'd object size#2570
BenWhitehead merged 6 commits into
mainfrom
fix/grpc-bidi-validation

Conversation

Copy link
Copy Markdown
Collaborator

Follow up to #2527 and #2514

The tests added in this PR account for ~940 of the added lines, however their scenarios are the same as the other upload scenario tests just using the different proto types. The remaining ~300 lines are the fix and small refactor to get the tests passing. I've left individual commits in the PR to make it easier to drill down if needed.

  • Part 1, add tests and simulation server

  • Cleanup unused inner-class

  • Move request trimming inline rather than at the end

  • Update GapicBidiUnbufferedWritableByteChannel to correctly handle failure scenarios {1, 2, 3, 4, 4_1, 4_2, 5, 7}

    Refactor ResumableSessionFailureScenario to accept Message and List rather than WriteObjectResponse and List.

    The shape of BidiWriteObjectRequest/WriteObjectRequest and BidiWriteObjectResponse/WriteObjectResponse are largely the same (bidi has two extra fields) so rather than overloading toStorageException yet again, this time there is a single method for grpc messages that internally can branch when formatting the request. In this case, we're quite safe taking this relaxed typing because it is an internal API that is only called from a grpc context where protos will be used.

  • Update GapicBidiUnbufferedWritableByteChannel to correctly handle partial consumption of content

    Tests passing now.

    Separate tracking of client detected errors and stream errors.

    When await is invoked, if a client detected error is present AND a stream error is present, the client detected error will take precedence. If a stream error happens and the client detected error has not yet been observed, the stream error will be added as a suppressed exception to the client detected error.

Follow up to #2527

Part 1, add tests and simulation server
Follow up to #2527

Move request trimming inline rather than at the end
Follow up to #2527

Update GapicBidiUnbufferedWritableByteChannel to correctly handle failure scenarios {1, 2, 3, 4, 4_1, 4_2, 5, 7}

Refactor ResumableSessionFailureScenario to accept Message and List rather than WriteObjectResponse and List<WriteObjectRequest>.

The shape of BidiWriteObjectRequest/WriteObjectRequest and BidiWriteObjectResponse/WriteObjectResponse are largely the same (bidi has two extra fields) so rather than overloading toStorageException yet again, this time there is a single method for grpc messages that internally can branch when formatting the request. In this case, we're quite safe taking this relaxed typing because it is an internal API that is only called from a grpc context where protos will be used.
Follow up to #2527

Update GapicBidiUnbufferedWritableByteChannel to correctly handle partial consumption of content

Tests passing now.

Separate tracking of client detected errors and stream errors.

When await is invoked, if a client detected error is present AND a stream error is present, the client detected error will take precedence. If a stream error happens and the client detected error has not yet been observed, the stream error will be added as a suppressed exception to the client detected error.
BenWhitehead added the owlbot:ignore instruct owl-bot to ignore a PR label May 31, 2024
BenWhitehead requested a review from a team May 31, 2024 16:54
product-auto-label Bot added size: xl Pull request size is extra large. api: storage Issues related to the googleapis/java-storage API. labels May 31, 2024
BenWhitehead force-pushed the fix/grpc-bidi-validation branch from 82427cc to f046ba1 Compare May 31, 2024 21:38
JesseLovelace self-assigned this Jun 11, 2024
BenWhitehead merged commit 5c9cecf into main Jun 13, 2024
BenWhitehead deleted the fix/grpc-bidi-validation branch June 13, 2024 21:55
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the googleapis/java-storage API. owlbot:ignore instruct owl-bot to ignore a PR size: xl Pull request size is extra large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL