-
Notifications
You must be signed in to change notification settings - Fork 138
refactor: consolidate BigQuery client creation and set user-agent #100
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
Conversation
project_id = default_project | ||
|
||
return bigquery.Client( | ||
client_info=google_client_info(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add hook to check if client_info
is available before trying to add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, 1.12.0 was released 2019-05-16. That's close enough to the NEP 29 recommended 2 years, that I think it's worth just bumping the minimum version of google-cloud-bigquery
in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor comment: I feel like I've seen/heard multiple conversations about resolving default project recently. I'd love it if this is going to go the route of ADC that we're consistent about the lookup order across languages and clients.
@shollyman re: default project lookup, most of the hard work is done by We don't hit that line if someone specifies a service account key, in which case we use the project ID from the JSON key file. That's consistent with what the |
Creating as draft, as I want to add unit tests first.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #98 🦕