Skip to content
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

feat(bigquery): add timeout parameter to QueryJob.done() method #9875

Merged
merged 6 commits into from
Dec 19, 2019

Conversation

plamut
Copy link
Contributor

@plamut plamut commented Nov 21, 2019

Fixes #7831.
Requires #9873.

This is a proposed fix for the done() method getting stuck. It makes sure that the underlying HTTP request does not block indefinitely when retrieving query results, and that any underlying requests exceptions are retriable.

TODO

@plamut plamut added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. api: bigquery Issues related to the BigQuery API. labels Nov 21, 2019
@plamut plamut requested a review from tswast November 21, 2019 15:01
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 21, 2019
bigquery/google/cloud/bigquery/client.py Outdated Show resolved Hide resolved
bigquery/google/cloud/bigquery/retry.py Outdated Show resolved Hide resolved
@plamut
Copy link
Contributor Author

plamut commented Dec 9, 2019

@tswast Ditched the custom HTTP adapter, and leveraged the new connection timeout parameter from #9915. The changes are now minimal, and can be easily applied to other client methods that would benefit from a timeout.

@plamut plamut force-pushed the iss-7831 branch 6 times, most recently from 87d74f7 to 2044a84 Compare December 11, 2019 07:48
@plamut plamut added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 11, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 11, 2019
@plamut
Copy link
Contributor Author

plamut commented Dec 11, 2019

A snippets test failed, seems like flakiness (it passed on several previous runs). Re-running.

In addition, fix the timeout logic in QueryJob.done() - the timeouts
are in different units (seconds vs. milliseconds)
The new timeout feature requires more recent versions of the API core
and google auth dependencies.
@plamut plamut marked this pull request as ready for review December 17, 2019 11:05
@plamut plamut requested review from a team and tswast December 17, 2019 11:05
bigquery/google/cloud/bigquery/job.py Outdated Show resolved Hide resolved
If the server-side processing timeout is used (the `timeout_ms` API
parameter) as the total timeout, it should be slightly longer than
the actual server-side timeout in order to not timeout the connection
while there might still be chance that the server-side processing
has actually completed.
@plamut plamut requested a review from tswast December 18, 2019 23:45
Copy link
Contributor

@tswast tswast left a comment

Choose a reason for hiding this comment

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

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BigQuery: QueryJob().done() method gets stuck
4 participants