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

BigQuery: Set BQ storage client user-agent when in Jupyter cell #8734

Merged
merged 5 commits into from
Jul 30, 2019

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Jul 23, 2019

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 subsequent comment about adding the user-agent info to the other client, too.

@plamut plamut added the api: bigquery Issues related to the BigQuery API. label Jul 23, 2019
@plamut plamut requested review from tswast and a team July 23, 2019 13:11
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 23, 2019
@plamut
Copy link
Contributor Author

plamut commented Jul 23, 2019

When running unit tests locally, I noticed that one of them fails here. The reason seems to be that float("nan") is converted to None by pyarrow, instead of the expected float("nan"). It also happens on the latest master, thus the failure seems unrelated to this PR.

My Pandas and pyarrow versions:

pandas==0.24.2
pyarrow==0.13.0

Update: The same happened in Kokoro checks, did not observe this yesterday...

bigquery/google/cloud/bigquery/magics.py Show resolved Hide resolved
"""Provide a patcher that can make the bigquery storage import to fail."""
orig_import = six.moves.builtins.__import__

def custom_import(name, globals=None, locals=None, fromlist=(), level=0):
Copy link
Contributor

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 (at https://github.com/googleapis/google-cloud-python/tree/master/test_utils/test_utils or maybe https://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.

Copy link
Contributor Author

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)

Copy link
Contributor Author

@plamut plamut Jul 26, 2019

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)

Copy link
Contributor

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.

@plamut plamut force-pushed the iss-8696-b branch 3 times, most recently from c1b1afc to b694a91 Compare July 26, 2019 13:01
The client is a grpc client, thus the grpc ClientInfo class should be
used to configure it.
@plamut plamut requested a review from tswast July 29, 2019 10:44
@plamut plamut added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 29, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 29, 2019
@plamut plamut requested a review from tswast July 30, 2019 06:23
@tswast tswast merged commit bb353b9 into googleapis:master Jul 30, 2019
@plamut plamut deleted the iss-8696-b branch July 31, 2019 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BigQuery: Add user-agent info to Jupyter magics Client constructor(s).
5 participants