Add Future interface to BigQuery jobs by theacodes · Pull Request #3626 · googleapis/google-cloud-python · GitHub

Resolves #3556
Towards #3617

@tswast this is the usage Im going for: https://gist.github.com/jonparrott/cb0ec50b8c70dfb32693fae021659baa

I don't like

query_job.result()
rows = query_job.results().fetch_data(max_results=10)

I'd prefer if it was

rows = query_job.result()

Also, the query job needs to be calling getQueryResults and checking for jobComplete in the response rather than polling the job state. The reason for this is that getQueryResults will wait for the job to complete or timeout (whichever comes first), whereas get Job returns immediately.

I'd prefer if it was
rows = query_job.result()

That seems reasonable as long as we don't lose data, but wouldn't that require switching to core iterator (#2840)? Could we accept the non-breaking behavior of this PR as-is for now and do another PR to switch the return value of result after #2840 is implemented?

Also, the query job needs to be calling getQueryResults and checking for jobComplete in the response rather than polling the job state. The reason for this is that getQueryResults will wait for the job to complete or timeout (whichever comes first), whereas get Job returns immediately.

Why is that preferable? Doing a long-lived HTTP request is quite different from any of the other behavior in this library. Also, would you be okay with addressing this in a follow-up PR rather than in this one?

#2840 is already implemented.

getQueryResults is preferable because the job polling method adds up to polling_interval milliseconds of latency to get the query results compared to the long-lived request which will return results as soon as they are available.

I'm okay with doing the getQueryResults in a subsequent PR.

@tswast Cool, are you good with this PR as-is then?

Could we make the change where QueryJob.result() returns QueryResults in this PR?

Could we make the change where QueryJob.result() returns QueryResults in this PR?

If not, I'll propose we make that as a breaking change when changing the polling method.

Could we make the change where QueryJob.result() returns QueryResults in this PR?

Yep. Done. Can you try this out and let me know how broken it is?

This should work:

def async_query(query):
    client = bigquery.Client()
    query_job = client.run_async_query(str(uuid.uuid4()), query)
    query_job.use_legacy_sql = False

    rows = query_job.result().fetch_data(max_results=10)
    for row in rows:
        print(row)

@tswast verified that this seems to work:

>>> from google.cloud import bigquery
>>> import uuid
>>>
>>> client = bigquery.Client()
>>> query = 'SELECT 1'
>>> query_job = client.run_async_query(str(uuid.uuid4()), query)
>>> query_job.use_legacy_sql = False
>>>
>>> rows = query_job.result().fetch_data(max_results=10)
>>> for row in rows:
...     print(row)
...
(1,)

I have no real concerns here though I think someone else should have a look?

LGTM

Thanks @dhermes and @tswast. This is a huge jump in developer experience for bigquery. 🍰

* Add future interface to bigquery Jobs. * Make QueryJob return QueryResults from result() * Deprecate QueryJob.results()

theacodes added api: bigquery Issues related to the BigQuery API. DevEx do not merge Indicates a pull request not ready for merge, due to either quality or timing. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Jul 18, 2017

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 18, 2017

tswast reviewed Jul 19, 2017

View reviewed changes

Comment thread core/google/cloud/future/base.py

retry_on = tenacity.retry_if_result(

functools.partial(operator.is_not, True))

# Use exponential backoff with jitter.

wait_on = (

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

theacodes force-pushed the bigquery-future branch from ee506c6 to e80335f Compare July 19, 2017 17:25

tswast approved these changes Jul 19, 2017

View reviewed changes

theacodes changed the title [DO NOT MERGE] Add Future interface to BigQuery jobs Add Future interface to BigQuery jobs Jul 20, 2017

Jon Wayne Parrott added 4 commits July 20, 2017 12:49

Add basic future interface to bigquery

cd0d443

Make QueryJob return QueryResults from result()

adec1f9

Fix supercall

d105de0

Finish tests

45f5400

theacodes force-pushed the bigquery-future branch from cf49707 to 45f5400 Compare July 20, 2017 20:59

theacodes requested review from dhermes and lukesneeringer July 20, 2017 21:00

dhermes reviewed Jul 20, 2017

View reviewed changes

Jon Wayne Parrott added 2 commits July 21, 2017 10:43

Address review comments

5c0b96f

Map errors to exceptions

8ee3af2

dhermes reviewed Jul 21, 2017

View reviewed changes

Jon Wayne Parrott added 2 commits July 21, 2017 11:36

Address review comments

f9a71ba

Fix exception formatting, add system test

7667c23

theacodes removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 21, 2017

dhermes approved these changes Jul 21, 2017

View reviewed changes

theacodes merged commit 6336e6c into googleapis:master Jul 21, 2017

theacodes deleted the bigquery-future branch July 21, 2017 22:42

landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 21, 2017

Add Future interface to BigQuery jobs (googleapis#3626 )

ead17f8

* Add future interface to bigquery Jobs. * Make QueryJob return QueryResults from result() * Deprecate QueryJob.results()

landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017

Add Future interface to BigQuery jobs (googleapis#3626 )

a76bb1d

* Add future interface to bigquery Jobs. * Make QueryJob return QueryResults from result() * Deprecate QueryJob.results()

landrito pushed a commit to landrito/google-cloud-python that referenced this pull request Aug 22, 2017

Add Future interface to BigQuery jobs (googleapis#3626 )

ff1e4fe

* Add future interface to bigquery Jobs. * Make QueryJob return QueryResults from result() * Deprecate QueryJob.results()

               from google.cloud.bigquery._helpers import UDFResourcesProperty
               from google.cloud.bigquery._helpers import _EnumProperty
               from google.cloud.bigquery._helpers import _TypedProperty
+              import google.cloud.future.base

                       api_response = client._connection.api_request(
                           method='POST', path='%s/cancel' % (self.path,))
                       self._set_properties(api_response['job'])
+                      return True

+                          # set, do not call set_result/set_exception again.
+                          # Note: self._result_set is set to True in set_result and
+                          # set_exception, in case those methods are invoked directly.
+                          if self.state != 'DONE' or self._result_set:

+                      """
+                      # Do not refresh is the state is already done, as the job will not
+                      # change once complete.
+                      if self.state != 'DONE':

+                  # make_exception expects an httplib2 response object.
+                  fake_response = _FakeResponse(status=status_code)
+                  return exceptions.make_exception(
+                      fake_response, b'', error_info=error_result, use_json=False)

+                      retry_on = tenacity.retry_if_result(
+                          functools.partial(operator.is_not, True))
+                      # Use exponential backoff with jitter.
+                      wait_on = (

+                          if self.error_result is not None:
+                              exception = exceptions.GoogleCloudError(
+                                  self.error_result, errors=self.errors)

                       return QueryResults.from_query_job(self)
+                  def results(self):
+                      """DEPRECATED.

+                  'tableUnavailable': http_client.BAD_REQUEST,
+              }
+              _FakeResponse = collections.namedtuple('_FakeResponse', ['status'])

                       """
-                      return False
+                      return (self.error_result is not None
+                              and self.error_result.get('reason') == 'stopped')

		import unittest


		class TestErrorResultToException(unittest.TestCase):



		def _error_result_to_exception(error_result):
		""""""

Add Future interface to BigQuery jobs by theacodes · Pull Request #3626 · googleapis/google-cloud-python · GitHub

Uh oh!

Add Future interface to BigQuery jobs - #3626

Add Future interface to BigQuery jobs#3626theacodes merged 8 commits intogoogleapis:mastergoogleapis/google-cloud-python:masterfrom theacodes:bigquery-futuretheacodes/google-cloud-python:bigquery-futureCopy head branch name to clipboard

Conversation

theacodes commented Jul 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theacodes commented Jul 18, 2017

Uh oh!

tswast commented Jul 19, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

theacodes commented Jul 19, 2017

Uh oh!

tswast commented Jul 19, 2017

Uh oh!

tswast commented Jul 19, 2017

Uh oh!

theacodes commented Jul 19, 2017

Uh oh!

tswast commented Jul 19, 2017

Uh oh!

tswast commented Jul 19, 2017

Uh oh!

theacodes commented Jul 19, 2017

Uh oh!

theacodes commented Jul 19, 2017

Uh oh!

dhermes left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Add Future interface to BigQuery jobs#3626
theacodes merged 8 commits into
googleapis:masterfrom
theacodes:bigquery-future

theacodes commented Jul 18, 2017 •
edited
Loading