Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: add BigQuery storage client support to DB API#36

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

Merged
plamut merged 10 commits intogoogleapis:masterfromplamut:iss-16
Feb 25, 2020

Conversation

@plamut
Copy link
Contributor

@plamutplamut commentedFeb 12, 2020
edited
Loading

Closes#16.

The title says it all. Details in the ticket description.

PR checklist:

  • Make sure to open an issue as abug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

@plamutplamut added the type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design. labelFeb 12, 2020
@googlebotgooglebot added the cla: yesThis human has signed the Contributor License Agreement. labelFeb 12, 2020
Copy link
Contributor

@tswasttswast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

General structure LGTM. Replacingself._query_data with an iterable that calls the BQ Storage API under the covers is exactly what I imagined.

Once the PR is ready, let's have@shollyman or someone he delegates to do the final review.

@plamut
Copy link
ContributorAuthor

plamut commentedFeb 14, 2020
edited
Loading

The coverage failure is due to Python 2.7 unit tests are not run on Kokoro:

Session unit-2.7 skipped: Python interpreter 2.7 not found.

The tests pass just fine locally, and the total coverage is 100%.

@plamutplamut marked this pull request as ready for reviewFebruary 14, 2020 14:15
@tseavertseaver self-requested a reviewFebruary 18, 2020 18:16
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment@googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️Googlers:Go here for more info.

1 similar comment
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment@googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️Googlers:Go here for more info.

@googlebotgooglebot added cla: noThis human has *not* signed the Contributor License Agreement. and removed cla: yesThis human has signed the Contributor License Agreement. labelsFeb 19, 2020
@plamutplamut requested a review fromtseaverFebruary 19, 2020 10:50
@plamut
Copy link
ContributorAuthor

@tseaver As a co-author of one of the changes, please just appease the CLA bot, thanks!

Copy link
Contributor

@shollymanshollyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for this. We'll want to update this from v1beta1->v1 in the near future, not sure how you'd like to track that.

@plamut
Copy link
ContributorAuthor

We'll want to update this from v1beta1->v1 in the near future, not sure how you'd like to track that.

@shollyman Track in the sense of the tasks that need to be done for the promotion to GA? A GitHub issue for it would be just fine.

@plamut
Copy link
ContributorAuthor

@tseaver Friendly ping, the CLA bot still needs confirmation from you. Thanks!

@tseaver
Copy link
Contributor

@googlebot I signed it!

@shollymanshollyman added cla: yesThis human has signed the Contributor License Agreement. and removed cla: noThis human has *not* signed the Contributor License Agreement. labelsFeb 25, 2020
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️Googlers:Go here for more info.

@plamutplamut merged commitba9b2f8 intogoogleapis:masterFeb 25, 2020
@plamutplamut deleted the iss-16 branchFebruary 25, 2020 18:47
@yan-hic
Copy link

Can usage for this be updated athttps://github.com/mxmzdlv/pybigquery/blob/master/README.rst#usage ?
Not sure how to enable storage API when loading in superset.

@yan-hic
Copy link

To be closed as pergoogleapis/python-bigquery-sqlalchemy#61 (comment) ?

@plamut
Copy link
ContributorAuthor

@yiga2 Sorry, I'm not following? What should be closed, this PR has been merged?

@yan-hic
Copy link

My bad - meant to closegoogleapis/python-bigquery-sqlalchemy#41

plamut reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@shollymanshollymanshollyman approved these changes

@tswasttswastAwaiting requested review from tswast

+1 more reviewer

@tseavertseavertseaver approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yesThis human has signed the Contributor License Agreement.type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BigQuery, BigQuery Storage]: Add option to use BigQuery Storage API to download results in BigQuery DB-API

6 participants

@plamut@googlebot@tseaver@yan-hic@tswast@shollyman

[8]ページ先頭

©2009-2025 Movatter.jp