Skip to content
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

feat: Add universe domain support for Java #1904

Merged
merged 11 commits into from
Feb 28, 2024
Merged

feat: Add universe domain support for Java #1904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b433095
chore: change assignees for issues and PRs to michaelpri10
michaelpri10 Feb 7, 2024
83b6ec3
Merge branch 'googleapis:main' into main
michaelpri10 Feb 7, 2024
0af60d7
feat: Add setUniverseDomain option for Publisher and Subscriber
michaelpri10 Feb 9, 2024
e4c1070
Merge branch 'googleapis:main' into tpc-java-support
michaelpri10 Feb 9, 2024
678a3aa
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Feb 8, 2024
bf48660
Merge branch 'main' into tpc-java-support
michaelpri10 Feb 12, 2024
9b37268
feat: Set default endpoint as null for Subscribers and Publishers
michaelpri10 Feb 12, 2024
9aec34d
chore: Remove unneeded sample snippet
michaelpri10 Feb 12, 2024
15b2498
Merge branch 'main' of https://github.com/michaelpri10/java-pubsub
michaelpri10 Feb 13, 2024
bf01a35
Merge remote-tracking branch 'refs/remotes/origin/main'
michaelpri10 Feb 14, 2024
bd35f3d
Merge branch 'main' into tpc-java-support
michaelpri10 Feb 14, 2024
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
10 changes: 9 additions & 1 deletion google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/Publisher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ private Publisher(Builder builder) throws IOException {
.setExecutorProvider(FixedExecutorProvider.create(executor))
.setTransportChannelProvider(builder.channelProvider)
.setEndpoint(builder.endpoint)
.setUniverseDomain(builder.universeDomain)
.setHeaderProvider(builder.headerProvider);
stubSettings
.publishSettings()
Expand Down Expand Up @@ -717,7 +718,8 @@ public static final class Builder {
static final long DEFAULT_COMPRESSION_BYTES_THRESHOLD = 240L;

String topicName;
private String endpoint = PublisherStubSettings.getDefaultEndpoint();
Copy link
Member

Choose a reason for hiding this comment

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

Is this still being used somewhere? I think it's autogenerated so it can't be removed IIRC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line was added when endpoint overriding was enabled for the Publisher/Subscriber here. If a user sets the endpoint when building the Publisher, we propagate that to the PublisherStubSettings builder, which is autogenerated. Otherwise, PublisherStubSettings uses the getDefaultEndpoint() method.

Copy link
Contributor

@lqiu96 lqiu96 Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

To add a bit more context from the GAPICs perspective. Gax-Java now will resolve the endpoint automatically so there is no need to set a default endpoint. One of the parameters that we take into consideration is the user set endpoint which is configured from the Builder's setEndpoint(). All these factors (i.e. mtls, user set endpoint, user set universe domain, and more) will be considered when creating the final endpoint to be used.

The reason why we're suggesting to not pass the getDefaultEndpoint to Gax is that Gax-Java won't be able to determine if the endpoint passed in is a user configuration or not. The new endpoint logic has user configuration override everything so it will always default to pubsub.googleapis.com if it is passed in.

private String endpoint = null;
private String universeDomain = null;

// Batching options
BatchingSettings batchingSettings = DEFAULT_BATCHING_SETTINGS;
Expand Down Expand Up @@ -857,6 +859,12 @@ public Builder setEndpoint(String endpoint) {
return this;
}

/** Gives the ability to override the universe domain. */
public Builder setUniverseDomain(String universeDomain) {
this.universeDomain = universeDomain;
return this;
}

/** Gives the ability to enable transport compression. */
public Builder setEnableCompression(boolean enableCompression) {
this.enableCompression = enableCompression;
Expand Down
10 changes: 9 additions & 1 deletion google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/Subscriber.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ private Subscriber(Builder builder) {
.setTransportChannelProvider(channelProvider)
.setHeaderProvider(builder.headerProvider)
.setEndpoint(builder.endpoint)
.setUniverseDomain(builder.universeDomain)
.build();
// TODO(pongad): what about internal header??
} catch (Exception e) {
Expand Down Expand Up @@ -491,7 +492,8 @@ public static final class Builder {
SubscriptionAdminSettings.defaultCredentialsProviderBuilder().build();
private Optional<ApiClock> clock = Optional.absent();
private int parallelPullCount = 1;
private String endpoint = SubscriberStubSettings.getDefaultEndpoint();
private String endpoint = null;
private String universeDomain = null;

Builder(String subscription, MessageReceiver receiver) {
this.subscription = subscription;
Expand Down Expand Up @@ -670,6 +672,12 @@ public Builder setEndpoint(String endpoint) {
return this;
}

/** Gives the ability to override the universe domain. */
public Builder setUniverseDomain(String universeDomain) {
this.universeDomain = universeDomain;
return this;
}

/** Gives the ability to set a custom clock. */
Builder setClock(ApiClock clock) {
this.clock = Optional.of(clock);
Expand Down
Loading