-
Notifications
You must be signed in to change notification settings - Fork 122
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
ENH: Add timeout support #76
Conversation
Codecov Report
@@ Coverage Diff @@
## master #76 +/- ##
===========================================
- Coverage 73.44% 28.18% -45.27%
===========================================
Files 4 4
Lines 1544 1554 +10
===========================================
- Hits 1134 438 -696
- Misses 410 1116 +706
Continue to review full report at Codecov.
|
Test results
|
Ummm, I cannot fix 3 fails of tests. |
Also, I have run tests on the master branch. Same 3 tests are failed. |
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.
Just one minor observation. Otherwise, looks good to me.
Could you add an entry to the change log under 0.2.1?
https://github.com/pydata/pandas-gbq/blob/master/docs/source/changelog.rst
pandas_gbq/gbq.py
Outdated
@@ -536,6 +543,11 @@ def run_query(self, query, **kwargs): | |||
|
|||
while not query_reply.get('jobComplete', False): | |||
self.print_elapsed_seconds(' Elapsed', 's. Waiting...') | |||
|
|||
timeoutMs = job_config['query'].get('timeoutMs') |
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.
timeoutMs
-> timeout_ms
to be consistent with existing code
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.
Make sense, I'll fix it.
@hagino3000 I've created #76 and #78 to address the unit test failures. Please feel free to add additional information. |
@parthea I added an entry to change log, and fix the variable name. |
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. Thanks @hagino3000 !
I'm going to make a minor adjustment to the change log before merging to mention that QueryTimeout will be raised when the specified timeout has expired.
pandas-gbq seems to wait for query results forever.
In rare cases, query does not finish long time (e.g. in 60 minutes) although the same query always finishes in few minutes.
I added timeout handling using configuration.query.timeoutMs.
https://cloud.google.com/bigquery/docs/reference/rest/v2/jobs/query
Example
Output