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.

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

Change pubsub endpoint from pubsub-experimental to pubsub #1149

Merged
merged 1 commit into from
Aug 10, 2016

Conversation

mziccard
Copy link
Contributor

@mziccard mziccard commented Aug 10, 2016

This change will be also made at the Veneer Toolkit level at next code-generation round. While we wait for that we go ahead and override the endpoint.

For reference: pubsub-experimental.googleapis.com stopped working today with the following error:

com.google.cloud.pubsub.PubSubException: io.grpc.StatusRuntimeException: PERMISSION_DENIED: Google Cloud Pub/Sub API (Experimental) has not been used in project google.com:gcloud-devel before or it is disabled. Enable it by visiting https://console.developers.google.com/apis/api/pubsub-experimental.googleapis.com/overview?project=gcloud-devel then retry. If you enabled this API recently, wait a few minutes for the action to propagate to our systems and retry.

See this comment and this StackOverflow question.

@mziccard mziccard added the api: pubsub Issues related to the Pub/Sub API. label Aug 10, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 10, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.009%) to 85.864% when pulling 8959fec on mziccard:fix-pubsub-endpoint into 0637c80 on GoogleCloudPlatform:master.

@mziccard
Copy link
Contributor Author

Please @lesv can you have a look at this?

@mziccard
Copy link
Contributor Author

@garrettjonesgoogle would you also mind having a quick look?

@garrettjonesgoogle
Copy link
Member

Why not just edit the line in PublisherSettings & SubscriberSettings with the default host?

@mziccard
Copy link
Contributor Author

Why not just edit the line in PublisherSettings & SubscriberSettings with the default host?

This way we also support setting other hosts using the host() setter in the option builder (even though it should not be needed). And I wanted to avoid conflicts with the autogen code.

.provideCredentialsWith(
credentials.createScoped(PublisherSettings.DEFAULT_SERVICE_SCOPES))
.build();
ConnectionSettings subConnectionSettings = ConnectionSettings.newBuilder()

This comment was marked as spam.

@garrettjonesgoogle
Copy link
Member

Why not just edit the line in PublisherSettings & SubscriberSettings with the default host?

This way we also support setting other hosts using the host() setter in the option builder (even though it should not be needed). And I wanted to avoid conflicts with the autogen code.

Ok it makes sense to plumb through the host from the options. But, my preference would be for PubSubOptions.DEFAULT_HOST to be set to PublisherSettings.DEFAULT_SERVICE_ADDRESS.

If you simply edit the DEFAULT_SERVICE_ADDRESS value e.g. at https://github.com/GoogleCloudPlatform/gcloud-java/blob/master/gcloud-java-pubsub/src/main/java/com/google/cloud/pubsub/spi/v1/PublisherSettings.java#L112 , then it should match what the generator will output in the next generation, and there won't be a conflict.

That said, it's not a strong opinion, and we can edit the veneer code when we generate next time.

@aozarov aozarov merged commit b25c462 into googleapis:master Aug 10, 2016
@garrettjonesgoogle
Copy link
Member

LGTM since this issue needs to be fixed urgently

@teddziuba
Copy link

Confirmed this fixes #1150. I hand built 0.2.7-SNAPSHOT and deployed it in my app. Looking forward to the next release.

@trentonstrong
Copy link

trentonstrong commented Aug 12, 2016

Thanks for fixing this so quickly! Appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants