- Notifications
You must be signed in to change notification settings - Fork 321
feat: use BigQuery Storage client by default #55
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
Conversation
25572ab to 1bb7283 Compare | I did some timings to compare fetching data with and without the BQ Storage client, results below. Spoiler: When using the BQ Storage API, performance improvements are enormous. QuerySource tables
Timings
|
fde9441 to c01648d Compare f080813 to 1aa0c62 Compare 1aa0c62 to 3f73a43 Compare 3f73a43 to 00bf584 Compare 12b2171 to dbef14d 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.
Semantic satiation seeing all the v1/v1beta references. Sorry it took me so long to get to this. Couple minor nits, with a couple more interesting bits (avro vs arrow, small result set, versioning for storage).
5b82c76 to c715a74 Compare * feat: add HOUR support for time partitioning interval
486309f to 6e35d09 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.
Thanks for the slogging through this.
| Just a thought - since this is a pretty significant change, how about releasing all the other smaller changes and fixes that have accumulated since the last release separately, and release this one separately for an easier rollback, should that be necessary? |
| Isolating this change to its own release seems prudent. |
| Putting this on hold until the next release is made, we will ship it separately. |
| With the new release (1.25.0) now out, we can now merge this and release in the near future (possibly with miscellaneous related cleanup fixes). |
Closes #86.
Closes #84.
No issue yet, just a POC preview.The PR is now ready to be reviewed. It uses the BQ Storage API by default to fetch query results, and removes the "beta" label from that feature.
It's probably easier to review this PR commit by commit.
Key points: