Skip to content

feat(gax): retry start upload call and rename ResumableUploadCallSettings to ResumableUploadOptions - #14509

Closed
blakeli0 wants to merge 1 commit into
googleapis:mainfrom
blakeli0:feat/resumable-upload-start-retry
Closed

blakeli0 wants to merge 1 commit into
googleapis:mainfrom
blakeli0:feat/resumable-upload-start-retry

Conversation

@blakeli0

@blakeli0 blakeli0 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Stacked PRs:


Summary

  • Rename ResumableUploadCallSettings to ResumableUploadOptions: Decouples per-RPC invocation options (chunkSize and globalTimeout) from method-level CallSettings so ResumableUploadOptions is strictly passed per-call to ResumableUploadCallable.futureCall(...) / resumeCall(...) and convenience upload methods.
  • Accept UnaryCallSettings in Callables.resumableUpload, HttpJsonCallableFactory.createResumableUploadCallable, HttpJsonResumableUploadClient.create, and ResumableUploadStartCallable.create: Uses UnaryCallSettings<RequestT, ?> in HttpJsonResumableUploadClient and ResumableUploadStartCallable (without extra overloads) and configures retryable codes and retry settings for the initial start upload HTTP call (X-Goog-Upload-Command: start).
  • Retry ResumableUploadStartCallable: Wraps the raw ResumableUploadStartCallable in HttpJsonExceptionCallable (with callSettings.getRetryableCodes()) and Callables.retrying(callable, callSettings, clientContext) so transient failures during session initiation are retried automatically.
  • Default globalTimeout from UnaryCallSettings: Callables.resumableUpload derives the default ResumableUploadOptions globalTimeout from callSettings.getRetrySettings().getTotalTimeoutDuration().

Related PRs

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the resumable upload configuration by replacing ResumableUploadCallSettings with ResumableUploadOptions for invocation-level options, and utilizing standard UnaryCallSettings for method-level settings. This change simplifies the settings hierarchy and affects client generation, stub settings, and the HTTP/JSON transport layer. The review feedback suggests enhancing type safety in HttpJsonResumableUploadClient and ResumableUploadStartCallable by replacing wildcard generic types with specific type parameters matching the request and response types.

public static <RequestT, ResponseT> HttpJsonResumableUploadClient<RequestT, ResponseT> create(
ClientContext clientContext,
ApiMethodDescriptor<RequestT, ResponseT> methodDescriptor,
UnaryCallSettings<?, ?> callSettings) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using UnaryCallSettings<RequestT, ResponseT> instead of the wildcard UnaryCallSettings<?, ?> improves type safety by ensuring that the call settings match the request and response types of the client.

Suggested change
UnaryCallSettings<?, ?> callSettings) {
UnaryCallSettings<RequestT, ResponseT> callSettings) {

ClientContext clientContext, ApiMethodDescriptor<RequestT, ResponseT> methodDescriptor) {
ClientContext clientContext,
ApiMethodDescriptor<RequestT, ResponseT> methodDescriptor,
UnaryCallSettings<?, ?> callSettings) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using UnaryCallSettings<RequestT, ResponseT> instead of the wildcard UnaryCallSettings<?, ?> improves type safety by ensuring that the call settings match the request and response types of the client.

Suggested change
UnaryCallSettings<?, ?> callSettings) {
UnaryCallSettings<RequestT, ResponseT> callSettings) {

static <RequestT> UnaryCallable<RequestT, ResumableUploadSession> create(
ClientContext clientContext,
ApiMethodDescriptor<RequestT, String> descriptor,
UnaryCallSettings<?, ?> callSettings) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using UnaryCallSettings<RequestT, ?> instead of UnaryCallSettings<?, ?> binds the request type of the settings to the request type of the callable, improving type safety.

Suggested change
UnaryCallSettings<?, ?> callSettings) {
UnaryCallSettings<RequestT, ?> callSettings) {

@blakeli0
blakeli0 force-pushed the feat/resumable-upload-start-retry branch from 45aa2e8 to ef2660a Compare September 25, 2026 05:58
@blakeli0 blakeli0 closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant