-
Notifications
You must be signed in to change notification settings - Fork 321
chore: add private _query_and_wait_bigframes
method
#2250
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
Towards internal issue b/409104302
827b81c
to
de668d6
Compare
Type failures:
I didn't touch these samples or Edit: Actually, I did add some type annotations where they were missing before. I'll see what needs to change. Edit2: Fixed! I reverted the type annotations changes for now. |
# TODO(tswast): After | ||
# https://github.com/googleapis/python-bigquery/pull/2260 goes in, add | ||
# created, started, ended properties here. |
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.
#BLOCK
PR #2260 is merged.
Please resolve the #TODO here and throughout the code (It shows up multiple times.)
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.
There are two blockers:
- It appears that a #TODO comment that appears multiple times in multiple files is no longer needed. Please resolve throughout the PR.
- There is word that has been incorrectly spelled (~ five times), please resolve.
Beyond that, this LGTM.
Hit me up as soon as these changes are made and I will Approve.
*
tests/unit/test_client_bigframes.py
Outdated
"totalBytesProcessed": "123", | ||
"totalSlotMs": "987", | ||
"jobComplete": True, | ||
# TODO(tswast): After |
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.
#BLOCK
Please resolve #TODO comment throughout this file (there are at least two references). PR 2260 is merged.
Co-authored-by: Chalmer Lowe <[email protected]>
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
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕