Skip to content
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

fix: broken create_job method #300

Merged
tswast merged 13 commits into from
Oct 20, 2020
Merged

fix: broken create_job method #300

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f305f04
fix: broken create_job method
HemangChothani Oct 6, 2020
f98950a
Merge branch 'master' of https://github.com/googleapis/python-bigquer…
HemangChothani Oct 7, 2020
38ce75e
fix: changes in unit tests
HemangChothani Oct 7, 2020
8a27c7c
Merge branch 'master' of https://github.com/googleapis/python-bigquer…
HemangChothani Oct 8, 2020
14bbe01
fix: fix sourceTable thing
HemangChothani Oct 12, 2020
64549c4
Merge branch 'master' of https://github.com/googleapis/python-bigquer…
HemangChothani Oct 12, 2020
f94998f
fix: handle sourceTable passed in job resource
HemangChothani Oct 13, 2020
d8b9e77
Merge branch 'master' of https://github.com/googleapis/python-bigquer…
HemangChothani Oct 13, 2020
b578c90
fix: resolve conflict and address comments
HemangChothani Oct 16, 2020
e130f9a
fix: remove delete destination table from query
HemangChothani Oct 19, 2020
5c7a39b
Merge branch 'master' of https://github.com/googleapis/python-bigquer…
HemangChothani Oct 19, 2020
4223b78
fix: revert destination table for query
HemangChothani Oct 19, 2020
104294a
Merge branch 'master' of https://github.com/googleapis/python-bigquer…
HemangChothani Oct 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions google/cloud/bigquery/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1616,6 +1616,7 @@ def create_job(self, job_config, retry=DEFAULT_RETRY):
)
destination = _get_sub_prop(job_config, ["load", "destinationTable"])
source_uris = _get_sub_prop(job_config, ["load", "sourceUris"])
destination = TableReference.from_api_repr(destination)
return self.load_table_from_uri(
source_uris, destination, job_config=load_job_config, retry=retry
)
Expand All @@ -1624,9 +1625,9 @@ def create_job(self, job_config, retry=DEFAULT_RETRY):
job_config
)
destination = _get_sub_prop(job_config, ["copy", "destinationTable"])
destination = TableReference.from_api_repr(destination)

@bhtucker bhtucker Oct 6, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would you like to guard this check with isinstance(TableReference, destination)?

@HemangChothani HemangChothani Oct 7, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I mock further in test so now it cover to_api_repr. I am not sure about the guard to check instance because job_config is dict and as per the example it has values in the string format.

sources = []
source_configs = _get_sub_prop(job_config, ["copy", "sourceTables"])

if source_configs is None:
source_configs = [_get_sub_prop(job_config, ["copy", "sourceTable"])]
Comment thread
tswast marked this conversation as resolved.
for source_config in source_configs:
Expand All @@ -1640,10 +1641,13 @@ def create_job(self, job_config, retry=DEFAULT_RETRY):
job_config
)
source = _get_sub_prop(job_config, ["extract", "sourceTable"])
source_type = "Table"
if not source:
if source:
source_type = "Table"
source = TableReference.from_api_repr(source)
else:
source = _get_sub_prop(job_config, ["extract", "sourceModel"])
source_type = "Model"
source = ModelReference.from_api_repr(source)
destination_uris = _get_sub_prop(job_config, ["extract", "destinationUris"])
return self.extract_table(
source,
Expand Down
63 changes: 34 additions & 29 deletions tests/unit/test_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3573,21 +3573,34 @@ def test_delete_table_w_not_found_ok_true(self):

conn.api_request.assert_called_with(method="DELETE", path=path, timeout=None)

def _create_job_helper(self, job_config, client_method):
def _create_job_helper(self, job_config):
from google.cloud.bigquery import _helpers

creds = _make_credentials()
http = object()
client = self._make_one(project=self.PROJECT, credentials=creds, _http=http)

client._connection = make_connection()
rf1 = mock.Mock()
get_config_patch = mock.patch(
"google.cloud.bigquery.job._JobConfig.from_api_repr", return_value=rf1,
)
load_patch = mock.patch(client_method, autospec=True)
RESOURCE = {
"jobReference": {"projectId": self.PROJECT, "jobId": mock.ANY},
"configuration": job_config,
}
conn = client._connection = make_connection(RESOURCE)
client.create_job(job_config=job_config)

with load_patch as client_method, get_config_patch:
client.create_job(job_config=job_config)
client_method.assert_called_once()
# if "copy" in job_config:

@tswast tswast Oct 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this commented out code needed?

@HemangChothani HemangChothani Oct 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, my bad.

# if "sourceTable" in job_config["copy"]:
# source = _helpers._get_sub_prop(job_config, ["copy", "sourceTable"])
# _helpers._del_sub_prop(job_config, ["copy", "sourceTable"])
# _helpers._set_sub_prop(job_config, ["copy", "sourceTables"], [source])
if "query" in job_config:
_helpers._del_sub_prop(job_config, ["query", "destinationTable"])
Comment thread
tswast marked this conversation as resolved.

conn.api_request.assert_called_once_with(
method="POST",
path="/projects/%s/jobs" % self.PROJECT,
data=RESOURCE,
timeout=None,
)

def test_create_job_load_config(self):
configuration = {
Expand All @@ -3601,9 +3614,7 @@ def test_create_job_load_config(self):
}
}

self._create_job_helper(
configuration, "google.cloud.bigquery.client.Client.load_table_from_uri"
)
self._create_job_helper(configuration)

def test_create_job_copy_config(self):
configuration = {
Expand All @@ -3623,9 +3634,7 @@ def test_create_job_copy_config(self):
}
}

self._create_job_helper(
configuration, "google.cloud.bigquery.client.Client.copy_table",
)
self._create_job_helper(configuration)

def test_create_job_copy_config_w_single_source(self):
Comment thread
tswast marked this conversation as resolved.
configuration = {
Expand All @@ -3643,9 +3652,7 @@ def test_create_job_copy_config_w_single_source(self):
}
}

self._create_job_helper(
configuration, "google.cloud.bigquery.client.Client.copy_table",
)
self._create_job_helper(configuration)

def test_create_job_extract_config(self):
configuration = {
Expand All @@ -3658,9 +3665,7 @@ def test_create_job_extract_config(self):
"destinationUris": ["gs://test_bucket/dst_object*"],
}
}
self._create_job_helper(
configuration, "google.cloud.bigquery.client.Client.extract_table",
)
self._create_job_helper(configuration)

def test_create_job_extract_config_for_model(self):
configuration = {
Expand All @@ -3673,17 +3678,17 @@ def test_create_job_extract_config_for_model(self):
"destinationUris": ["gs://test_bucket/dst_object*"],
}
}
self._create_job_helper(
configuration, "google.cloud.bigquery.client.Client.extract_table",
)
self._create_job_helper(configuration)

def test_create_job_query_config(self):
configuration = {
"query": {"query": "query", "destinationTable": {"tableId": "table_id"}}
"query": {
"query": "query",
"destinationTable": {"tableId": "table_id"},
"useLegacySql": False,
}
}
self._create_job_helper(
configuration, "google.cloud.bigquery.client.Client.query",
)
self._create_job_helper(configuration)

def test_create_job_query_config_w_rateLimitExceeded_error(self):
from google.cloud.exceptions import Forbidden
Expand Down