-
Notifications
You must be signed in to change notification settings - Fork 7
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
Remove destination table from BigQuery config #171
Remove destination table from BigQuery config #171
Conversation
f2f863c
to
fa7d824
Compare
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.
Nit: currently the PR is a one line change but there are 2 commits; better squash them into 1.
Besides that, this LGTM, but I must admit I'm not sure I get its purpose 😇 could you elaborate a bit on the context/why?
Thanks!
fa7d824
to
156b513
Compare
@codificat thanks for the review, I updated the PR description and squashed the commits |
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
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: harshad16 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This introduces a breaking change
This should yield a new module release
This Pull Request implements
Remove BigQuery destination table in
pypi-downloads
handler. Writing to a destination table does not seem to have any utility here and might be the source of a lot of traffic generated on the BigQuery API (~6000 calls to thegoogle.cloud.bigquery.v2.JobService.InsertJob
API method).