-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
BigQuery: Set IPython user agent when running queries with IPython cell magic #8713
Conversation
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.
LGTM.
Test failures might be from an update to a dependency (pyarrow) that causes more warnings to be raised than expected.
We should see what those new warnings are and update the test to not care quite so much about them if they're safe to ignore.
A connection from the context could override this custom user agent string - is that expected behavior, or should the cell magic actually override the user agent string in context connections, too?
We added that _connection
override feature for Kaggle public datasets, since they proxy connections to BigQuery. We have other ways to determine which requests come from Kaggle, and if anyone is proxying like Kaggle is doing, arguably we should keep whatever user-agent they have set, as it will be more specific than ours.
Any additional warnings on top of the expected pyarrow warning would cause one of the tests to fail. This commit makes that test more robust by only focusing on the existence of the specific warning of interest.
The three new deprecation warnings that caused one of the test to fail were related to using some deprecated Pandas thingies, namely the I modified the test to only focus on the expected warning related to |
Heh, now the coverage check is complaining. Will check and make the necessary adjustments. |
The coverage check complained, because not all paths have been taken in the However, I did notice that even those three new warnings are caused by "{message : DeprecationWarning('RangeIndex._start is deprecated and will be removed in a future version. Use RangeIndex.start instead',), category :
'DeprecationWarning', filename : '/home/.../bigquery/.nox/unit-3-6/lib/python3.6/site-packages/pyarrow/pandas_compat.py', lineno : 383, line : None}" |
a6c8a88
to
7b6aecc
Compare
Closes #8696.
How to test
See issue description - the Client should be instantiated with a custom user agent string when used from an IPython cell.
Things to discuss
A connection from the context could override this custom user agent string - is that expected behavior, or should the cell magic actually override the user agent string in context connections, too?No, this is primarily used with Kaggle datasets, thus the overridden user agent string should take precedence.