Uh oh!
There was an error while loading. Please reload this page.
feat: add job_timeout_ms to job configuration classes - #1672
feat: add job_timeout_ms to job configuration classes#1672DevStephanie wants to merge 2 commits into
job_timeout_ms to job configuration classes#1672Conversation
cea062c to
3c9b034Compare
Slight tidy up.
3c9b034 to
ec045b5Comparejob_timeout_ms to job configuration classesUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| ValueError: If ``value`` type is invalid. | ||
| """ | ||
| # None as this is an optional parameter. |
There was a problem hiding this comment.
It could be None, but it could also be a real integer value. We should return self._properties.get("jobTimeoutMs") (converted to an integer if not None. I believe there are other integer properties that use a helper function for this conversion.)
| @jobtimeout_ms.setter | ||
| def jobtimeout(self, value): | ||
| if not isinstance(value, int): |
| return None | ||
| @jobtimeout_ms.setter | ||
| def jobtimeout(self, value): |
There was a problem hiding this comment.
Make sure this function name matches your @property.
| from google.cloud.bigquery import _helpers | ||
| from google.cloud.bigquery import standard_sql | ||
| from google.cloud.bigquery.encryption_configuration import EncryptionConfiguration | ||
| import google.cloud.bigquery.model |
There was a problem hiding this comment.
We can revert the changes in this file.
| self.assertEqual(job.ddl_target_table.project, self.PROJECT) | ||
| def test_jobtimeout_ms(self): | ||
| self.assertIsInstance(job.timeout_ms, value) |
There was a problem hiding this comment.
Let's set the job configuration timeout ms to something before asserting the value.
There was a problem hiding this comment.
Also, move these tests to where we test QueryJobConfig: https://github.com/googleapis/python-bigquery/blob/main/tests/unit/job/test_query_config.py#L20
Uh oh!
There was an error while loading. Please reload this page.
| assert object_under_test.labels =={} | ||
| object_under_test.labels = {"data_owner": "someteam"} | ||
| assert object_under_test.labels == {"data_owner": "someteam"} | ||
| assert object_under_test.labels() =={} |
chalmerlowe
commented
Oct 5, 2023
Thanks for providing this PR. I am grateful that you are looking to assist. For the record: work is being conducted in the following PR to address this issue. |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕