Skip to content

Conversation

@daspecster
Copy link
Contributor

@daspecster daspecster commented Oct 28, 2016

Usually prefix is passed to def _compute_type_url(klass, prefix) so this should be a low/no impact change.

However I don't see where Bigtable passes prefix...

@daspecster daspecster added api: bigtable Issues related to the Bigtable API. api: speech Issues related to the Speech-to-Text API. labels Oct 28, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 28, 2016
@tseaver
Copy link
Contributor

tseaver commented Oct 28, 2016

@daspecster The _GOOGLE_APIS_PREFIX constant is only used as the default value for the prefix argument to _compute_type_url. Outside its own tests, that helper is called only to generate type URLs for Bigtable's UpdateClusterMetadata and CreateClusterMetadata: the effect of borking the URL is that callers won't be able to fetch convert the metadata for those operations.

@tseaver
Copy link
Contributor

tseaver commented Oct 28, 2016

LGTM

@daspecster
Copy link
Contributor Author

@tseaver it doesn't look like there are system tests for Bigtable Clusters?
I checked the type_urls for the Bigtable system tests but I'm just realizing that there are no cluster type_urls when I looked before.

I also don't see CreateClusterMetadata in the docs https://cloud.google.com/bigtable/docs/reference/admin/rpc/google.bigtable.admin.v2#google.bigtable.admin.v2.CreateClusterRequest.

@daspecster
Copy link
Contributor Author

I'm going to take your LGTM though. Googling for types.googleapis.com comes up with another typo I believe.

@daspecster daspecster merged commit 8038698 into googleapis:master Oct 28, 2016
@daspecster daspecster deleted the update-pb-types-prefix branch October 28, 2016 18:14
@dhermes
Copy link
Contributor

dhermes commented Oct 28, 2016

@daspecster It'd help if you posted a snippet that verified for Bigtable, similar to https://gist.github.com/dhermes/09c964d6d27003ae817b650424fda7c3 for speech

@daspecster
Copy link
Contributor Author

Ok, I'll see what I can do for that.

@dhermes
Copy link
Contributor

dhermes commented Oct 28, 2016

It doesn't have to be complex / overly reproducible. Whatever you used to show me

response {
 type_url: "type.googleapis.com/google.bigtable.admin.v2.Instance"
 value: "\n2projects/ferrous-arena/instances/new-1477676796867\022\021new-1477676796867\030\001"
}

from before will be fine.

@daspecster
Copy link
Contributor Author

I put import pdb; pdb.set_trace() after this line
https://github.com/GoogleCloudPlatform/google-cloud-python/blob/40932e9307066987bf7e325bbc87b267d6d22f0e/core/google/cloud/operation.py#L104
and checked the value of type_value from running tox -e system-test -- bigtable.

@dhermes
Copy link
Contributor

dhermes commented Oct 28, 2016

Ha, I like it!

@daspecster
Copy link
Contributor Author

😄

richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
…efix

Actual type_url URI is type.googleapis.com.
parthea pushed a commit that referenced this pull request Nov 24, 2025
Actual type_url URI is type.googleapis.com.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API. api: speech Issues related to the Speech-to-Text API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants