Skip to content

feat(generator): emit ResumableUploadCallSettings on stub and service settings - #14318

Draft
whowes wants to merge 1 commit into
whowes/generator-allowlist-parserfrom
whowes/generator-settings-codegen
Draft

feat(generator): emit ResumableUploadCallSettings on stub and service settings#14318
whowes wants to merge 1 commit into
whowes/generator-allowlist-parserfrom
whowes/generator-settings-codegen

Conversation

@whowes

@whowes whowes commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Work in progress, not ready for review

@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 adds support for generating ResumableUploadCallSettings in the Java GAPIC generator. It updates the settings and stub settings composers to detect resumable upload methods, generate corresponding call settings, and apply global timeout configurations from the service config. The review feedback suggests replacing !Objects.isNull with a standard null check and guarding against a potential NullPointerException when fetching retry settings.

}

if (method.isResumableUpload()) {
if (!Objects.isNull(serviceConfig)) {

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

Use the standard and idiomatic serviceConfig != null null-check instead of !Objects.isNull(serviceConfig). This is more readable and avoids unnecessary dependency on java.util.Objects.

Suggested change
if (!Objects.isNull(serviceConfig)) {
if (serviceConfig != null) {

Comment on lines +1611 to +1614
GapicRetrySettings retrySettings =
serviceConfig
.getAllGapicRetrySettings(service)
.get(serviceConfig.getRetryParamsName(service, method));

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

If serviceConfig.getRetryParamsName(service, method) returns null (e.g., when the method is not configured in the service config), calling .get(null) on the map returned by getAllGapicRetrySettings can throw a NullPointerException (especially if the map is a Guava ImmutableMap). Guard against this by checking if the retry params name is null first.

Suggested change
GapicRetrySettings retrySettings =
serviceConfig
.getAllGapicRetrySettings(service)
.get(serviceConfig.getRetryParamsName(service, method));
String retryParamsName = serviceConfig.getRetryParamsName(service, method);
GapicRetrySettings retrySettings = retryParamsName != null
? serviceConfig.getAllGapicRetrySettings(service).get(retryParamsName)
: null;

@whowes
whowes added this pull request to stack #14327 September 9, 2026 06:35
@whowes
whowes force-pushed the whowes/generator-settings-codegen branch from 9e50315 to e99b1f1 Compare September 9, 2026 15:50
@whowes
whowes force-pushed the whowes/generator-settings-codegen branch from e99b1f1 to 14d6389 Compare September 9, 2026 20:16
@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

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