- Notifications
You must be signed in to change notification settings - Fork321
fix: invalid conversion of timezone-aware datetime values to JSON#480
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
| def_timestamp_to_json_row(value): | ||
| """Coerce 'value' to an JSON-compatible representation.""" | ||
| ifisinstance(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. Thestrftime() conversion just uses thedatetime fields at their face value and ignores anytzinfo 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 naivedatetime 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: