From eee398a94418e002f4f7df6ba1f9c8793cb2a627 Mon Sep 17 00:00:00 2001 From: Jon Wayne Parrott Date: Mon, 24 Jul 2017 13:52:46 -0700 Subject: [PATCH 1/2] Remove QueryJob.results() --- .../google/cloud/bigquery/dbapi/_helpers.py | 21 ----------- .../google/cloud/bigquery/dbapi/cursor.py | 9 +++-- bigquery/google/cloud/bigquery/job.py | 16 -------- bigquery/tests/unit/test_dbapi__helpers.py | 37 ------------------- bigquery/tests/unit/test_dbapi_cursor.py | 20 +++++++++- bigquery/tests/unit/test_job.py | 11 ------ 6 files changed, 25 insertions(+), 89 deletions(-) diff --git a/bigquery/google/cloud/bigquery/dbapi/_helpers.py b/bigquery/google/cloud/bigquery/dbapi/_helpers.py index 1a9a02fd7cc7..a9a358cbf0f5 100644 --- a/bigquery/google/cloud/bigquery/dbapi/_helpers.py +++ b/bigquery/google/cloud/bigquery/dbapi/_helpers.py @@ -15,7 +15,6 @@ import collections import datetime import numbers -import time import six @@ -23,26 +22,6 @@ from google.cloud.bigquery.dbapi import exceptions -def wait_for_job(job): - """Waits for a job to complete by polling until the state is `DONE`. - - Sleeps 1 second between calls to the BigQuery API. - - :type job: :class:`~google.cloud.bigquery.job._AsyncJob` - :param job: Wait for this job to finish. - - :raises: :class:`~google.cloud.bigquery.dbapi.exceptions.DatabaseError` - if the job fails. - """ - while True: - job.reload() - if job.state == 'DONE': - if job.error_result: - raise exceptions.DatabaseError(job.errors) - return - time.sleep(1) - - def scalar_to_query_parameter(value, name=None): """Convert a scalar value into a query parameter. diff --git a/bigquery/google/cloud/bigquery/dbapi/cursor.py b/bigquery/google/cloud/bigquery/dbapi/cursor.py index bcbb19cfd066..ff5a701cab11 100644 --- a/bigquery/google/cloud/bigquery/dbapi/cursor.py +++ b/bigquery/google/cloud/bigquery/dbapi/cursor.py @@ -17,6 +17,7 @@ import collections import uuid +import google.cloud.exceptions import six from google.cloud.bigquery.dbapi import _helpers @@ -148,9 +149,11 @@ def execute(self, operation, parameters=None): formatted_operation, query_parameters=query_parameters) query_job.use_legacy_sql = False - query_job.begin() - _helpers.wait_for_job(query_job) - query_results = query_job.results() + + try: + query_results = query_job.result() + except google.cloud.exceptions.GoogleCloudError: + raise exceptions.DatabaseError(query_job.errors) # Force the iterator to run because the query_results doesn't # have the total_rows populated. See: diff --git a/bigquery/google/cloud/bigquery/job.py b/bigquery/google/cloud/bigquery/job.py index 35a423b755b9..3e6a9f93418b 100644 --- a/bigquery/google/cloud/bigquery/job.py +++ b/bigquery/google/cloud/bigquery/job.py @@ -16,7 +16,6 @@ import collections import threading -import warnings import six from six.moves import http_client @@ -1264,21 +1263,6 @@ def query_results(self): from google.cloud.bigquery.query import QueryResults return QueryResults.from_query_job(self) - def results(self): - """DEPRECATED. - - This method is deprecated. Use :meth:`query_results` or :meth:`result`. - - Construct a QueryResults instance, bound to this job. - - :rtype: :class:`~google.cloud.bigquery.query.QueryResults` - :returns: The query results. - """ - warnings.warn( - 'QueryJob.results() is deprecated. Please use query_results() or ' - 'result().', DeprecationWarning) - return self.query_results() - def result(self, timeout=None): """Start the job and wait for it to complete and get the result. diff --git a/bigquery/tests/unit/test_dbapi__helpers.py b/bigquery/tests/unit/test_dbapi__helpers.py index e030ed49df0c..48bca5ae9a59 100644 --- a/bigquery/tests/unit/test_dbapi__helpers.py +++ b/bigquery/tests/unit/test_dbapi__helpers.py @@ -16,48 +16,11 @@ import math import unittest -import mock - import google.cloud._helpers from google.cloud.bigquery.dbapi import _helpers from google.cloud.bigquery.dbapi import exceptions -class Test_wait_for_job(unittest.TestCase): - - def _mock_job(self): - from google.cloud.bigquery import job - mock_job = mock.create_autospec(job.QueryJob) - mock_job.state = 'RUNNING' - mock_job._mocked_iterations = 0 - - def mock_reload(): - mock_job._mocked_iterations += 1 - if mock_job._mocked_iterations >= 2: - mock_job.state = 'DONE' - - mock_job.reload.side_effect = mock_reload - return mock_job - - def _call_fut(self, job): - from google.cloud.bigquery.dbapi._helpers import wait_for_job - with mock.patch('time.sleep'): - wait_for_job(job) - - def test_wo_error(self): - mock_job = self._mock_job() - mock_job.error_result = None - self._call_fut(mock_job) - self.assertEqual('DONE', mock_job.state) - - def test_w_error(self): - from google.cloud.bigquery.dbapi import exceptions - mock_job = self._mock_job() - mock_job.error_result = {'reason': 'invalidQuery'} - self.assertRaises(exceptions.DatabaseError, self._call_fut, mock_job) - self.assertEqual('DONE', mock_job.state) - - class TestQueryParameters(unittest.TestCase): def test_scalar_to_query_parameter(self): diff --git a/bigquery/tests/unit/test_dbapi_cursor.py b/bigquery/tests/unit/test_dbapi_cursor.py index 9671a27b8f8f..2a2ccfd989a6 100644 --- a/bigquery/tests/unit/test_dbapi_cursor.py +++ b/bigquery/tests/unit/test_dbapi_cursor.py @@ -42,7 +42,7 @@ def _mock_job( mock_job = mock.create_autospec(job.QueryJob) mock_job.error_result = None mock_job.state = 'DONE' - mock_job.results.return_value = self._mock_results( + mock_job.result.return_value = self._mock_results( rows=rows, schema=schema, num_dml_affected_rows=num_dml_affected_rows) return mock_job @@ -219,6 +219,24 @@ def test_execute_w_query(self): row = cursor.fetchone() self.assertIsNone(row) + def test_execute_raises_if_result_raises(self): + import google.cloud.exceptions + + from google.cloud.bigquery import client + from google.cloud.bigquery import job + from google.cloud.bigquery.dbapi import connect + from google.cloud.bigquery.dbapi import exceptions + + job = mock.create_autospec(job.QueryJob) + job.result.side_effect = google.cloud.exceptions.GoogleCloudError('') + client = mock.create_autospec(client.Client) + client.run_async_query.return_value = job + connection = connect(client) + cursor = connection.cursor() + + with self.assertRaises(exceptions.DatabaseError): + cursor.execute('SELECT 1') + def test_executemany_w_dml(self): from google.cloud.bigquery.dbapi import connect connection = connect( diff --git a/bigquery/tests/unit/test_job.py b/bigquery/tests/unit/test_job.py index 8b9d079df148..fcb518d9c502 100644 --- a/bigquery/tests/unit/test_job.py +++ b/bigquery/tests/unit/test_job.py @@ -13,7 +13,6 @@ # limitations under the License. import copy -import warnings from six.moves import http_client import unittest @@ -1560,16 +1559,6 @@ def test_query_results(self): self.assertIsInstance(results, QueryResults) self.assertIs(results._job, job) - def test_results_is_deprecated(self): - client = _Client(self.PROJECT) - job = self._make_one(self.JOB_NAME, self.QUERY, client) - - with warnings.catch_warnings(record=True) as warned: - warnings.simplefilter('always') - job.results() - self.assertEqual(len(warned), 1) - self.assertIn('deprecated', str(warned[0])) - def test_result(self): from google.cloud.bigquery.query import QueryResults From e357914aa530aeff767197e8ce86f87adc9341d8 Mon Sep 17 00:00:00 2001 From: Jon Wayne Parrott Date: Mon, 24 Jul 2017 14:05:06 -0700 Subject: [PATCH 2/2] Move import --- bigquery/google/cloud/bigquery/dbapi/cursor.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bigquery/google/cloud/bigquery/dbapi/cursor.py b/bigquery/google/cloud/bigquery/dbapi/cursor.py index ff5a701cab11..7519c762ae1e 100644 --- a/bigquery/google/cloud/bigquery/dbapi/cursor.py +++ b/bigquery/google/cloud/bigquery/dbapi/cursor.py @@ -17,12 +17,11 @@ import collections import uuid -import google.cloud.exceptions import six from google.cloud.bigquery.dbapi import _helpers from google.cloud.bigquery.dbapi import exceptions - +import google.cloud.exceptions # Per PEP 249: A 7-item sequence containing information describing one result # column. The first two items (name and type_code) are mandatory, the other