- Notifications
You must be signed in to change notification settings - Fork1.6k
BigQuery: Set BQ storage client user-agent when in Jupyter cell#8734
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
plamut commentedJul 23, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
When running unit tests locally, I noticed that one of them failshere. The reason seems to be that My Update: The same happened in Kokoro checks, did not observe this yesterday... |
Uh oh!
There was an error while loading.Please reload this page.
bigquery/tests/unit/test_magics.py Outdated
| """Provide a patcher that can make the bigquery storage import to fail.""" | ||
| orig_import=six.moves.builtins.__import__ | ||
| defcustom_import(name,globals=None,locals=None,fromlist=(),level=0): |
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.
Should we make this__import__ mock into a test utility (athttps://github.com/googleapis/google-cloud-python/tree/master/test_utils/test_utils or maybehttps://github.com/googleapis/google-cloud-python/blob/master/bigquery/tests/unit/helpers.py)?
Seems generally useful, and also something I'd like to see tests for.
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.
I'd say we put it in bigquery test utils for now, and ask other stakeholders if they are fine with promoting it to core test utils.
(there might be additional features/generalizations requested, but that would not block this particular PR)
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.
BTW, whom can I ask if the new helper is fit for promotion, who owns the core?
(if there isn't one and we can do it at our own discretion, please let me know)
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.
BTW, whom can I ask if the new helper is fit for promotion, who owns the core?
(if there isn't one and we can do it at our own discretion, please let me know)
I would say add a separate PR promoting it, and tag@busunkim96 for review.
c1b1afc tob694a91CompareUh oh!
There was an error while loading.Please reload this page.
The client is a grpc client, thus the grpc ClientInfo class should beused to configure it.
Uh oh!
There was an error while loading.Please reload this page.
Closes#8696
This is a follow-up to#8713, it adds the same user-agent string to the optional BQ storage client, if the latter is used.
How to test
See issue description and the subsequentcomment about adding the user-agent info to the other client, too.