Repository navigation
feat(gax): add helpers to detect and surface unsupported resumable upload configurations on failure - #14652
feat(gax): add helpers to detect and surface unsupported resumable upload configurations on failure#14652whowes wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces ResumableUploadTransportValidator to validate transport configurations on StubSettings for resumable uploads, exposing helper methods via HttpJsonCallableFactory. A review comment suggests adding a defensive null check for the transport channel provider in ResumableUploadTransportValidator to prevent a potential NullPointerException.
whowes
changed the title
9ef8b1e to
5ecea13
Compare
whowes
changed the title
b7097e4 to
ea63e73
Compare
| // credentials, and endpoint configured on StubSettings. The provider is only compatible if it | ||
| // does not wrap a pre-built channel (FixedTransportChannelProvider) and still expects all | ||
| // three settings to be supplied by StubSettings. | ||
| return !(provider instanceof FixedTransportChannelProvider) |
There was a problem hiding this comment.
Discussed offline that this may introduce false positives. Because we are always creating a new channel from StubSettings, as long as customers set the same headers/credentials/endpoints on the StubSettings as well, resumable upload should be supported.
On the other hand, I don't think there is a concrete way to decide if a customized grpc channel can be supported for resumable uploads. This check still provide some value and can be used for a warning message instead of a hard failure.
There was a problem hiding this comment.
Updated so this no longer causes hard failures. If the original TransportChannelProvider was customized outside Settings, we'll append an actionable hint to any terminal upload exception.
whowes
changed the title
5494b0c to
85cef05
Compare
85cef05 to
80d36d4
Compare
|
|
| // credentials, and endpoint configured on StubSettings. The provider is only compatible if it | ||
| // does not wrap a pre-built channel (FixedTransportChannelProvider) and still expects all | ||
| // three settings to be supplied by StubSettings. | ||
| return !(provider instanceof FixedTransportChannelProvider) |
There was a problem hiding this comment.
Updated so this no longer causes hard failures. If the original TransportChannelProvider was customized outside Settings, we'll append an actionable hint to any terminal upload exception.
| .setDefaultCallContext( | ||
| uploadContext | ||
| .getDefaultCallContext() | ||
| .withOption(NON_SETTINGS_TRANSPORT_CUSTOMIZATION_KEY, true)) |
There was a problem hiding this comment.
Flagging the use of ApiCallContext.withOption here to make the customization status available to ResumableUploadFutureImpl.toTerminalException at call time.
Passing this explicitly would mean modifying public surface on three layers (HttpJsonCallableFactory → Callables → ResumableUploadCallableImpl). This is set once on the upload context's default call context, and only as a boolean when the provider was customized outside Settings. I think it's a reasonable use of withOption given its original intent (gax-java #1413, gax-java #1435), but LMK if you'd prefer a different approach.
This branch has not been deployed
2 participants
gRPC clients bridge resumable uploads to HTTP/JSON using the top-level StubSettings; endpoint, credentials, or header overrides on the TransportChannelProvider (or a pre-built channel) are currently silently ignored for uploads. In addition, it's currently possible for the upload-specific ClientContext to fail stub creation if it throws an exception, which does not meet the requirement that failures should be deferred until resumable upload methods are called.
This PR adds ResumableUploadTransportBridge for use by generated code, which includes:
If the original provider was customized outside Settings, terminal upload errors include a hint to configure the endpoint, credentials, and headers on Settings instead.