Skip to content

feat(Datastore): Swap the default transport from HttpJson to gRPC #12977

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

Open
lqiu96 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat(Datastore): Swap the default transport from HttpJson to gRPC #12977

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
80dcf14
feat(Datastore): Swap the default transport from HttpJson to gRPC
lqiu96 May 1, 2026
1550342
chore: Address PR comments
lqiu96 May 1, 2026
93ce220
chore: Add comments to explain the logic
lqiu96 May 1, 2026
6696b85
Merge branch 'main' into datastore-swap-default-transport
lqiu96 May 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions ...ore/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java
View file
Open in desktop
Comment thread
lqiu96 marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -131,21 +131,26 @@ public static class Builder extends ServiceOptions.Builder<Datastore, DatastoreO

@Nullable private DatastoreOpenTelemetryOptions openTelemetryOptions = null;

private Builder() {}
private Builder() {
this.transportOptions = new DatastoreDefaults().getDefaultTransportOptions();
}
Comment thread
lqiu96 marked this conversation as resolved.
Outdated

private Builder(DatastoreOptions options) {
super(options);
this.namespace = options.namespace;
this.databaseId = options.databaseId;
this.openTelemetryOptions = options.openTelemetryOptions;
this.channelProvider = validateChannelProvider(options.channelProvider);
this.host = options.getHost();
this.transportOptions = options.getTransportOptions();
Comment on lines 147 to +149
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 1, 2026

Choose a reason for hiding this comment

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

high

These assignments are redundant and potentially introduce a bug. The super(options) call already copies the host and transportOptions fields from the provided options object. By calling options.getHost() and options.getTransportOptions(), you are resolving the defaults and storing them as explicit values in the builder. This breaks the toBuilder() pattern where a user might want to change the transport and have the host automatically switch to its corresponding default. Per repository guidelines, reuse pre-configured objects from options classes directly instead of creating new ones and copying settings.

Suggested change
this.channelProvider = validateChannelProvider(options.channelProvider);
this.host = options.getHost();
this.transportOptions = options.getTransportOptions();
this.channelProvider = validateChannelProvider(options.channelProvider);
References
  1. Reuse pre-configured objects from options classes directly instead of creating new ones and copying settings.

}

@Override
public Builder setTransportOptions(TransportOptions transportOptions) {
if (!(transportOptions instanceof HttpTransportOptions)) {
if (!(transportOptions instanceof HttpTransportOptions)
&& !(transportOptions instanceof GrpcTransportOptions)) {
throw new IllegalArgumentException(
"Only http transport is allowed for " + API_SHORT_NAME + ".");
"Only http and grpc transport are allowed for " + API_SHORT_NAME + ".");
}
this.transportOptions = transportOptions;
return super.setTransportOptions(transportOptions);
Comment on lines 178 to 179
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 1, 2026

Choose a reason for hiding this comment

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

medium

The assignment this.transportOptions = transportOptions; is redundant because super.setTransportOptions(transportOptions) already performs this assignment in the base class.

Suggested change
this.transportOptions = transportOptions;
return super.setTransportOptions(transportOptions);
return super.setTransportOptions(transportOptions);

Expand Down Expand Up @@ -308,8 +313,8 @@ public TransportOptions getDefaultTransportOptions() {
return TRANSPORT_OPTIONS;
}

public static HttpTransportOptions.Builder getDefaultTransportOptionsBuilder() {
return HttpTransportOptions.newBuilder();
public static GrpcTransportOptions.Builder getDefaultTransportOptionsBuilder() {
return GrpcTransportOptions.newBuilder();
}
}

Expand Down
22 changes: 20 additions & 2 deletions ...google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,17 @@ public void testInvalidConfigForHttp() {

@Test
public void testTransport() {
// default http transport
assertThat(options.build().getTransportOptions()).isInstanceOf(HttpTransportOptions.class);
// default grpc transport
assertThat(options.build().getTransportOptions()).isInstanceOf(GrpcTransportOptions.class);

// custom http transport
DatastoreOptions httpTransportOptions =
DatastoreOptions.newBuilder()
.setTransportOptions(HttpTransportOptions.newBuilder().build())
.setProjectId(PROJECT_ID)
.setCredentials(NoCredentials.getInstance())
.build();
assertThat(httpTransportOptions.getTransportOptions()).isInstanceOf(HttpTransportOptions.class);

// custom grpc transport
DatastoreOptions grpcTransportOptions =
Expand Down Expand Up @@ -317,15 +326,24 @@ public void testHostWithGrpcAndHttp() {
.build();
assertThat(grpcTransportOptionsCustomHost.getHost()).isEqualTo(customHost);

DatastoreOptions defaultTransportOptions =
DatastoreOptions.newBuilder()
.setProjectId(PROJECT_ID)
.setCredentials(NoCredentials.getInstance())
.build();
assertThat(defaultTransportOptions.getHost()).isEqualTo(DatastoreSettings.getDefaultEndpoint());

DatastoreOptions httpTransportOptions =
DatastoreOptions.newBuilder()
.setTransportOptions(HttpTransportOptions.newBuilder().build())
.setProjectId(PROJECT_ID)
.setCredentials(NoCredentials.getInstance())
.build();
assertThat(httpTransportOptions.getHost()).isEqualTo(DatastoreFactory.DEFAULT_HOST);

DatastoreOptions httpTransportOptionsCustomHost =
DatastoreOptions.newBuilder()
.setTransportOptions(HttpTransportOptions.newBuilder().build())
.setHost(customHost)
.setProjectId(PROJECT_ID)
.setCredentials(NoCredentials.getInstance())
Expand Down
Loading