-
Notifications
You must be signed in to change notification settings - Fork 306
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.
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
fix: invalid conversion of timezone-aware datetime values to JSON #480
Conversation
@@ -315,13 +315,21 @@ def _timestamp_to_json_parameter(value): | |||
def _timestamp_to_json_row(value): | |||
"""Coerce 'value' to an JSON-compatible representation.""" | |||
if isinstance(value, datetime.datetime): | |||
# For naive datetime objects UTC timezone is assumed, thus we format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this always our assumption? Based on the unit tests, I suppose so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't say if "always", but the _datetime_to_json()
helper has been unchanged since 2017 and was supposedly doing its job fine with naive datetime objects in this time - at least I don't remember any reported issues related to timezones. The strftime()
conversion just uses the datetime
fields at their face value and ignores any tzinfo
set.
Considering that naive datetimes are used most of the time (as opposed to timezone-aware datetime objects) and that users did not complain, the UTC timezone assumption was apparently correct behavior. The logic just failed to cover the case when somebody actually used a timezone-aware instance.
And indeed, unit tests expect the same, a naive datetime
instance is supposed to be converted to a string representation with a zero offset from UTC (example).
🤖 I have created a release \*beep\* \*boop\* --- ## [2.7.0](https://www.github.com/googleapis/python-bigquery/compare/v2.6.2...v2.7.0) (2021-01-27) ### Bug Fixes * invalid conversion of timezone-aware datetime values to JSON ([#480](https://www.github.com/googleapis/python-bigquery/issues/480)) ([61b4385](https://www.github.com/googleapis/python-bigquery/commit/61b438523d305ce66a68fde7cb49e9abbf0a8d1d)) * reading the labels attribute on Job instances ([#471](https://www.github.com/googleapis/python-bigquery/issues/471)) ([80944f0](https://www.github.com/googleapis/python-bigquery/commit/80944f080bcc4fda870a6daf1d884de616d39ae7)) * use explicitly given project over the client's default project for load jobs ([#482](https://www.github.com/googleapis/python-bigquery/issues/482)) ([530e1e8](https://www.github.com/googleapis/python-bigquery/commit/530e1e8d8fe8939e914a78ff1b220907c1b87af7)) ### Dependencies * declare support for Python 3.9 ([#488](https://www.github.com/googleapis/python-bigquery/issues/488)) ([55daa7d](https://www.github.com/googleapis/python-bigquery/commit/55daa7da9857a8a2fb14a80a4efa3f466386a85f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Fixes #476.
This PR fixes converting timezone-aware datetime values to an UTC JSON string.
PR checklist: