Skip to content

Commit

Permalink
fix: Make the one breaking change required to allow regions on TopicP…
Browse files Browse the repository at this point in the history 
…ath or SubscriptionPath (#729)

* fix: Make the one breaking change required to allow regions on TopicPath or SubscriptionPath

* fix: Code should use "extractRegion" which is the location-ambiguous accessor.
  • Loading branch information
@dpcollins-google
dpcollins-google committed Jun 24, 2021
1 parent c860ae9 commit 5cf5783
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 37 deletions.
16 changes: 2 additions & 14 deletions google-cloud-pubsublite/clirr-ignored-differences.xml
View file
Expand Up @@ -4,28 +4,16 @@
<!-- TODO: Remove on next release -->
<difference>
<differenceType>7006</differenceType>
<className>com/google/cloud/pubsublite/LocationPath</className>
<className>com/google/cloud/pubsublite/SubscriptionPath</className>
<method>*</method>
<to>*</to>
</difference>
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/pubsublite/AdminClient</className>
<method>*</method>
</difference>
<difference>
<!-- This one is actually a clirr bug: It doesn't handle CRTP well. -->
<differenceType>7006</differenceType>
<className>com/google/cloud/pubsublite/LocationPath$Builder</className>
<className>com/google/cloud/pubsublite/TopicPath</className>
<method>*</method>
<to>*</to>
</difference>
<difference>
<!-- This one is actually a clirr bug: It doesn't handle CRTP well. -->
<differenceType>7002</differenceType>
<className>com/google/cloud/pubsublite/LocationPath$Builder</className>
<method>*</method>
</difference>
<!-- END TODO: Remove on next release -->
<!-- Added abstract method to AutoValue.Builder class (Always okay) -->
<difference>
Expand Down
6 changes: 4 additions & 2 deletions google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/PartitionLookupUtils.java
View file
Expand Up @@ -31,7 +31,7 @@ private PartitionLookupUtils() {}
public static int numPartitions(TopicPath topic) throws ApiException {
try (AdminClient client =
AdminClient.create(
AdminClientSettings.newBuilder().setRegion(topic.location().region()).build())) {
AdminClientSettings.newBuilder().setRegion(topic.location().extractRegion()).build())) {
return numPartitions(topic, client);
} catch (Exception e) {
throw ExtractStatus.toCanonical(e).underlying;
Expand Down Expand Up @@ -59,7 +59,9 @@ public static int numPartitions(TopicPath topic, AdminClient client) throws ApiE
public static int numPartitions(SubscriptionPath subscription) throws ApiException {
try (AdminClient client =
AdminClient.create(
AdminClientSettings.newBuilder().setRegion(subscription.location().region()).build())) {
AdminClientSettings.newBuilder()
.setRegion(subscription.location().extractRegion())
.build())) {
return numPartitions(subscription, client);
} catch (Throwable t) {
throw ExtractStatus.toCanonical(t).underlying;
Expand Down
12 changes: 9 additions & 3 deletions google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/SubscriptionPath.java
View file
Expand Up @@ -33,7 +33,7 @@
public abstract class SubscriptionPath implements Serializable {
public abstract ProjectIdOrNumber project();

public abstract CloudZone location();
public abstract CloudRegionOrZone location();

public abstract SubscriptionName name();

Expand All @@ -55,7 +55,12 @@ public static Builder newBuilder() {

@AutoValue.Builder
public abstract static class Builder extends ProjectBuilderHelper<Builder> {
public abstract Builder setLocation(CloudZone zone);
// TODO(dpcollins): Make this public and use ProjectLocationBuilderHelper once region is allowed
abstract Builder setLocation(CloudRegionOrZone location);

public Builder setLocation(CloudZone zone) {
return setLocation(CloudRegionOrZone.of(zone));
}

public abstract Builder setName(SubscriptionName name);

Expand All @@ -68,12 +73,13 @@ public static SubscriptionPath parse(String path) throws ApiException {
checkArgument(splits.length == 6);
checkArgument(splits[4].equals("subscriptions"));
LocationPath location = LocationPath.parse(String.join("/", Arrays.copyOf(splits, 4)));
// TODO(dpcollins): Remove once region is allowed
checkArgument(
location.location().getKind() == Kind.ZONE,
"Subscription location must be a valid cloud zone.");
return SubscriptionPath.newBuilder()
.setProject(location.project())
.setLocation(location.location().zone())
.setLocation(location.location())
.setName(SubscriptionName.of(splits[5]))
.build();
}
Expand Down
12 changes: 9 additions & 3 deletions google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/TopicPath.java
View file
Expand Up @@ -33,7 +33,7 @@
public abstract class TopicPath implements Serializable {
public abstract ProjectIdOrNumber project();

public abstract CloudZone location();
public abstract CloudRegionOrZone location();

public abstract TopicName name();

Expand All @@ -55,7 +55,12 @@ public static Builder newBuilder() {

@AutoValue.Builder
public abstract static class Builder extends ProjectBuilderHelper<Builder> {
public abstract Builder setLocation(CloudZone zone);
// TODO(dpcollins): Make this public and use ProjectLocationBuilderHelper once region is allowed
abstract Builder setLocation(CloudRegionOrZone location);

public Builder setLocation(CloudZone zone) {
return setLocation(CloudRegionOrZone.of(zone));
}

public abstract Builder setName(TopicName name);

Expand All @@ -68,11 +73,12 @@ public static TopicPath parse(String path) throws ApiException {
checkArgument(splits.length == 6);
checkArgument(splits[4].equals("topics"));
LocationPath location = LocationPath.parse(String.join("/", Arrays.copyOf(splits, 4)));
// TODO(dpcollins): Remove once region is allowed
checkArgument(
location.location().getKind() == Kind.ZONE, "Topic location must be a valid cloud zone.");
return TopicPath.newBuilder()
.setProject(location.project())
.setLocation(location.location().zone())
.setLocation(location.location())
.setName(TopicName.of(splits[5]))
.build();
}
Expand Down
6 changes: 3 additions & 3 deletions ...d-pubsublite/src/main/java/com/google/cloud/pubsublite/cloudpubsub/PublisherSettings.java
View file
Expand Up @@ -158,7 +158,7 @@ private PublisherServiceClient newServiceClient(Partition partition) throws ApiE
settingsBuilder);
try {
return PublisherServiceClient.create(
addDefaultSettings(topicPath().location().region(), settingsBuilder));
addDefaultSettings(topicPath().location().extractRegion(), settingsBuilder));
} catch (Throwable t) {
throw toCanonical(t).underlying;
}
Expand All @@ -172,10 +172,10 @@ private AdminClient getAdminClient() throws ApiException {
.setServiceClient(
AdminServiceClient.create(
addDefaultSettings(
topicPath().location().region(),
topicPath().location().extractRegion(),
AdminServiceSettings.newBuilder()
.setCredentialsProvider(credentialsProvider()))))
.setRegion(topicPath().location().region())
.setRegion(topicPath().location().extractRegion())
.build());
} catch (Throwable t) {
throw toCanonical(t).underlying;
Expand Down
6 changes: 3 additions & 3 deletions ...-pubsublite/src/main/java/com/google/cloud/pubsublite/cloudpubsub/SubscriberSettings.java
View file
Expand Up @@ -220,7 +220,7 @@ private SubscriberServiceClient newSubscriberServiceClient(Partition partition)
RoutingMetadata.of(subscriptionPath(), partition),
settingsBuilder);
return SubscriberServiceClient.create(
addDefaultSettings(subscriptionPath().location().region(), settingsBuilder));
addDefaultSettings(subscriptionPath().location().extractRegion(), settingsBuilder));
} catch (Throwable t) {
throw toCanonical(t).underlying;
}
Expand All @@ -233,7 +233,7 @@ private CursorServiceClient newCursorServiceClient() throws ApiException {
try {
return CursorServiceClient.create(
addDefaultSettings(
subscriptionPath().location().region(),
subscriptionPath().location().extractRegion(),
CursorServiceSettings.newBuilder().setCredentialsProvider(credentialsProvider())));
} catch (Throwable t) {
throw toCanonical(t).underlying;
Expand Down Expand Up @@ -282,7 +282,7 @@ private PartitionAssignmentServiceClient getAssignmentServiceClient() throws Api
try {
return PartitionAssignmentServiceClient.create(
addDefaultSettings(
subscriptionPath().location().region(),
subscriptionPath().location().extractRegion(),
PartitionAssignmentServiceSettings.newBuilder()
.setCredentialsProvider(credentialsProvider())));
} catch (Throwable t) {
Expand Down
6 changes: 3 additions & 3 deletions pubsublite-beam-io/src/main/java/com/google/cloud/pubsublite/beam/Publishers.java
View file
Expand Up @@ -51,9 +51,9 @@ private static AdminClient newAdminClient(PublisherOptions options) throws ApiEx
.setServiceClient(
AdminServiceClient.create(
addDefaultSettings(
options.topicPath().location().region(),
options.topicPath().location().extractRegion(),
AdminServiceSettings.newBuilder())))
.setRegion(options.topicPath().location().region())
.setRegion(options.topicPath().location().extractRegion())
.build());
} catch (Throwable t) {
throw toCanonical(t).underlying;
Expand All @@ -70,7 +70,7 @@ private static PublisherServiceClient newServiceClient(
settingsBuilder);
try {
return PublisherServiceClient.create(
addDefaultSettings(options.topicPath().location().region(), settingsBuilder));
addDefaultSettings(options.topicPath().location().extractRegion(), settingsBuilder));
} catch (Throwable t) {
throw toCanonical(t).underlying;
}
Expand Down
2 changes: 1 addition & 1 deletion pubsublite-beam-io/src/main/java/com/google/cloud/pubsublite/beam/SubscribeTransform.java
View file
Expand Up @@ -114,7 +114,7 @@ private TopicPath getTopicPath() {
try (AdminClient admin =
AdminClient.create(
AdminClientSettings.newBuilder()
.setRegion(options.subscriptionPath().location().region())
.setRegion(options.subscriptionPath().location().extractRegion())
.build())) {
return TopicPath.parse(admin.getSubscription(options.subscriptionPath()).get().getTopic());
} catch (Throwable t) {
Expand Down
6 changes: 3 additions & 3 deletions pubsublite-beam-io/src/main/java/com/google/cloud/pubsublite/beam/SubscriberOptions.java
View file
Expand Up @@ -119,7 +119,7 @@ private SubscriberServiceClient newSubscriberServiceClient(Partition partition)
RoutingMetadata.of(subscriptionPath(), partition),
settingsBuilder);
return SubscriberServiceClient.create(
addDefaultSettings(subscriptionPath().location().region(), settingsBuilder));
addDefaultSettings(subscriptionPath().location().extractRegion(), settingsBuilder));
} catch (Throwable t) {
throw toCanonical(t).underlying;
}
Expand Down Expand Up @@ -147,7 +147,7 @@ private CursorServiceClient newCursorServiceClient() throws ApiException {
try {
return CursorServiceClient.create(
addDefaultSettings(
subscriptionPath().location().region(), CursorServiceSettings.newBuilder()));
subscriptionPath().location().extractRegion(), CursorServiceSettings.newBuilder()));
} catch (Throwable t) {
throw toCanonical(t).underlying;
}
Expand Down Expand Up @@ -186,7 +186,7 @@ InitialOffsetReader getInitialOffsetReader(Partition partition) {
return new InitialOffsetReaderImpl(
CursorClient.create(
CursorClientSettings.newBuilder()
.setRegion(subscriptionPath().location().region())
.setRegion(subscriptionPath().location().extractRegion())
.build()),
subscriptionPath(),
partition);
Expand Down
6 changes: 4 additions & 2 deletions ...te-beam-io/src/main/java/com/google/cloud/pubsublite/beam/TopicBacklogReaderSettings.java
View file
Expand Up @@ -61,7 +61,7 @@ Builder setTopicPathFromSubscriptionPath(SubscriptionPath subscriptionPath)
try (AdminClient adminClient =
AdminClient.create(
AdminClientSettings.newBuilder()
.setRegion(subscriptionPath.location().region())
.setRegion(subscriptionPath.location().extractRegion())
.build())) {
return setTopicPath(
TopicPath.parse(adminClient.getSubscription(subscriptionPath).get().getTopic()));
Expand All @@ -80,7 +80,9 @@ Builder setTopicPathFromSubscriptionPath(SubscriptionPath subscriptionPath)

TopicBacklogReader instantiate() throws ApiException {
TopicStatsClientSettings settings =
TopicStatsClientSettings.newBuilder().setRegion(topicPath().location().region()).build();
TopicStatsClientSettings.newBuilder()
.setRegion(topicPath().location().extractRegion())
.build();
TopicBacklogReader impl =
new TopicBacklogReaderImpl(TopicStatsClient.create(settings), topicPath(), partition());
return new LimitingTopicBacklogReader(impl, Ticker.systemTicker());
Expand Down

0 comments on commit 5cf5783

Please sign in to comment.