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

feat: update synth to generate v1beta2, v1 endpoints for bigquerystorage #10

Merged
merged 7 commits into from Feb 21, 2020

Conversation

shollyman
Copy link
Contributor

@shollyman shollyman commented Feb 19, 2020

This PR contains both the changes to the synth config, as well as the generated output from running synth.

There's some work to try to accomodate v1alpha2 in this PR, but
there exists further oddities that must be tackled before we can
fully add generation of the alpha client here.

Also, note that this PR does not include manual client modifications
such as streaming offset resumption that are present in the v1beta1
client. Intent is to address further changes like that in subsequent
PRs.

There's also some work to try to accomodate v1alpha2 in this PR, but
there exists further oddities that must be tackled before we can
fully add generation of the alpha client here.

Also, note that this PR does not include manual client modifications
such as streaming offset resumption that are present in the v1beta1
client.  Intent is to address further changes like that in subsequent
PRs.
@googlebot googlebot added the cla: yes label Feb 19, 2020
@shollyman shollyman requested review from tswast, busunkim96 and plamut Feb 19, 2020
Copy link
Contributor

@tswast tswast left a comment

Changes to synth.py LGTM.

Are you planning to handle the handwritten "reader" module and manual client wrappers in a subsequent PR?

}
if version in ["v1beta2","v1"]:
clientinfo = {
"file": "big_query_read_client.py",
Copy link
Contributor

@tswast tswast Feb 19, 2020

Choose a reason for hiding this comment

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

Am I understanding this correctly that

  • v1beta2 and v1 only have a read client
  • v1alpha2 only has a write client
  • and v1beta1 only has a read client, but it's called a storage client?

Copy link
Contributor Author

@shollyman shollyman Feb 19, 2020

Choose a reason for hiding this comment

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

That's the gist of it, yes. I expect once v1alpha2 revs (e.g. to...v2beta1 or similar) we'll have both a read and write client in the same library/version.

from google.cloud.bigquery_storage_v1beta1 import BigQueryStorageClient
from google.cloud.bigquery_storage_v1beta1 import enums
from google.cloud.bigquery_storage_v1beta1 import types
from google.cloud.bigquery_storage_v1 import BigQueryReadClient
Copy link
Contributor Author

@shollyman shollyman Feb 19, 2020

Choose a reason for hiding this comment

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

@tswast This is one change I'm worried about. Do we rely on this non-versioned client for anything elsewhere that would care about getting shifted from v1beta1 to v1? It's going to require further changes to address if so.

Copy link
Contributor

@tswast tswast Feb 19, 2020

Choose a reason for hiding this comment

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

In our public samples, I always used the v1beta1 module. There are a few versions of pandas-gbq that use the versionless module, but I fixed that in the latest version. (googleapis/python-bigquery-pandas@e177978#diff-4db670026d33c02e5ad3dfbd5e4fd595R11)

So, yes: this will break a few versions of pandas-gbq. Might be worth making some aliases for BigQueryStorageClient and the necessary methods to avoid breakages.

Copy link
Contributor Author

@shollyman shollyman Feb 19, 2020

Choose a reason for hiding this comment

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

Given the proto messages have changed between v1beta1 and v1beta2 it seems you'd need more than just an alias to get the right client name. Things like tablereferences became a single string rather than a structured object, sharding strategy went away (everything is balanced), which makes converting a bit dicey if you were expecting the old strategy.

Copy link
Contributor

@tswast tswast Feb 19, 2020

Choose a reason for hiding this comment

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

Gotcha. When customers hit this, we can tell them to either upgrade pandas-gbq or pin their google-cloud-bigquery-storage version.

@shollyman shollyman merged commit 2ea5ac4 into googleapis:master Feb 21, 2020
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants