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.

Sign up for GitHub

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

feature: raise error for unknown properties in job config #446

Merged
merged 6 commits into from
Jan 13, 2021
Merged

feature: raise error for unknown properties in job config #446

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
995a73f
feature: warn about unknown properties in job config
cguardia Dec 22, 2020
3210f40
fix: raise error instead of warning
cguardia Jan 8, 2021
d2a8d05
fix: use hasattr instead of __dict__
cguardia Jan 10, 2021
2fe95b5
Merge branch 'master' into 303-unknown-properties
tswast Jan 12, 2021
f9c0b4c
fix bad merge
tswast Jan 12, 2021
10d6bb6
fix system test that sets wrong property
tswast Jan 13, 2021
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
6 changes: 6 additions & 0 deletions google/cloud/bigquery/job/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,12 @@ def __init__(self, job_type, **kwargs):
for prop, val in kwargs.items():
setattr(self, prop, val)

def __setattr__(self, name, value):
"""Override to be able to raise error if an unknown property is being set"""
if not name.startswith("_") and name not in type(self).__dict__:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's try using hasattr to see if that accounts for superclass properties.

Suggested change
if not name.startswith("_") and name not in type(self).__dict__:
if not name.startswith("_") and not hasattr(self, name):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, but we have to use type(self), because if not the properties are looked up on the instance.

raise AttributeError("Property {} is unknown for {}.".format(name, type(self)))
super(_JobConfig, self).__setattr__(name, value)

@property
def labels(self):
"""Dict[str, str]: Labels for the job.
Expand Down
7 changes: 7 additions & 0 deletions tests/unit/job/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from google.api_core import exceptions
import google.api_core.retry
import mock
import pytest
from six.moves import http_client

from .helpers import _make_client
Expand Down Expand Up @@ -1021,6 +1022,12 @@ def test_ctor(self):
self.assertEqual(job_config._job_type, self.JOB_TYPE)
self.assertEqual(job_config._properties, {self.JOB_TYPE: {}})

def test_ctor_with_unknown_property_raises_error(self):
error_text = "Property wrong_name is unknown for"
with pytest.raises(AttributeError, match=error_text):
config = self._make_one()
config.wrong_name = None

def test_fill_from_default(self):
from google.cloud.bigquery import QueryJobConfig

Expand Down