Skip to content

Commit

Permalink
Don't raise error when encountering unknown fields in Models API. (#8083
Browse files Browse the repository at this point in the history
)

As new fields are added, the JSON -> Protobuf conversion should not
fail. Instead, it should ignore unknown fields. So that this data is not
discarded, use `_properties` as is the convention in our REST libraries.
It's private, but can be used as a workaround to get access to fields
that haven't yet been added to the client library.
  • Loading branch information
tswast authored May 21, 2019
1 parent 42a05e5 commit 25b988f
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 7 deletions.
20 changes: 13 additions & 7 deletions bigquery/google/cloud/bigquery/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,9 @@ def from_api_repr(cls, resource):
google.cloud.bigquery.model.Model: Model parsed from ``resource``.
"""
this = cls(None)
# Keep a reference to the resource as a workaround to find unknown
# field values.
this._properties = resource

# Convert from millis-from-epoch to timestamp well-known type.
# TODO: Remove this hack once CL 238585470 hits prod.
Expand All @@ -279,12 +282,9 @@ def from_api_repr(cls, resource):
start_time = datetime_helpers.from_microseconds(1e3 * float(start_time))
training_run["startTime"] = datetime_helpers.to_rfc3339(start_time)

this._proto = json_format.ParseDict(resource, types.Model())
for key in six.itervalues(cls._PROPERTY_TO_API_FIELD):
# Leave missing keys unset. This allows us to use setdefault in the
# getters where we want a default value other than None.
if key in resource:
this._properties[key] = resource[key]
this._proto = json_format.ParseDict(
resource, types.Model(), ignore_unknown_fields=True
)
return this

def _build_resource(self, filter_fields):
Expand All @@ -304,6 +304,7 @@ class ModelReference(object):

def __init__(self):
self._proto = types.ModelReference()
self._properties = {}

@property
def project(self):
Expand Down Expand Up @@ -342,7 +343,12 @@ def from_api_repr(cls, resource):
Model reference parsed from ``resource``.
"""
ref = cls()
ref._proto = json_format.ParseDict(resource, types.ModelReference())
# Keep a reference to the resource as a workaround to find unknown
# field values.
ref._properties = resource
ref._proto = json_format.ParseDict(
resource, types.ModelReference(), ignore_unknown_fields=True
)
return ref

@classmethod
Expand Down
16 changes: 16 additions & 0 deletions bigquery/tests/unit/model/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,22 @@ def test_from_api_repr_w_minimal_resource(target_class):
assert len(got.label_columns) == 0


def test_from_api_repr_w_unknown_fields(target_class):
from google.cloud.bigquery import ModelReference

resource = {
"modelReference": {
"projectId": "my-project",
"datasetId": "my_dataset",
"modelId": "my_model",
},
"thisFieldIsNotInTheProto": "just ignore me",
}
got = target_class.from_api_repr(resource)
assert got.reference == ModelReference.from_string("my-project.my_dataset.my_model")
assert got._properties is resource


@pytest.mark.parametrize(
"resource,filter_fields,expected",
[
Expand Down
14 changes: 14 additions & 0 deletions bigquery/tests/unit/model/test_model_reference.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ def test_from_api_repr(target_class):
assert got.path == "/projects/my-project/datasets/my_dataset/models/my_model"


def test_from_api_repr_w_unknown_fields(target_class):
resource = {
"projectId": "my-project",
"datasetId": "my_dataset",
"modelId": "my_model",
"thisFieldIsNotInTheProto": "just ignore me",
}
got = target_class.from_api_repr(resource)
assert got.project == "my-project"
assert got.dataset_id == "my_dataset"
assert got.model_id == "my_model"
assert got._properties is resource


def test_to_api_repr(target_class):
ref = target_class.from_string("my-project.my_dataset.my_model")
got = ref.to_api_repr()
Expand Down

0 comments on commit 25b988f

Please sign in to comment.