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.

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

Timestamp values: Remove the use of "naive" Python datetime objects #667

Merged
merged 1 commit into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,20 @@ Unreleased
- The SQLAlchemy dialect has been split off into the `sqlalchemy-cratedb`_
package. See `Migrate from crate.client to sqlalchemy-cratedb`_ to learn
about necessary migration steps.
- Returned Python ``datetime`` objects are now always timezone-aware,
using UTC by default. This is a possible BREAKING CHANGE: Removed the use
of "naive" Python ``datetime`` objects, i.e. instances without ``tzinfo``
attribute set.
When no ``time_zone`` information is specified when creating a database
connection or cursor, ``datetime`` objects will now use Coordinated
Universal Time (UTC), like CrateDB is storing timestamp values in this
format.
This update is coming from a deprecation of Python's
``datetime.utcfromtimestamp()``, which is effectively also phasing out
the use of "naive" timestamp objects in Python, in favor of using
timezone-aware objects, also to represent datetimes in UTC. It may be a
breaking change for some users of the library that don't expect to
receive "aware" ``datetime`` objects from now on.
- Configured DB API interface attribute ``threadsafety = 1``, which signals
"Threads may share the module, but not connections."
- Added ``error_trace`` to string representation of an Error to relay
Expand Down
5 changes: 2 additions & 3 deletions docs/by-example/cursor.rst
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ types. Currently, this is implemented for the CrateDB data types ``IP`` and
>>> cursor.execute('')

>>> cursor.fetchone()
['foo', IPv4Address('10.10.10.1'), datetime.datetime(2022, 7, 18, 18, 10, 36, 758000)]
['foo', IPv4Address('10.10.10.1'), datetime.datetime(2022, 7, 18, 18, 10, 36, 758000, tzinfo=datetime.timezone.utc)]


Custom data type conversion
Expand Down Expand Up @@ -374,8 +374,7 @@ Proof that the converter works correctly, ``B\'0110\'`` should be converted to
=======================================

Based on the data type converter functionality, the driver offers a convenient
interface to make it return timezone-aware ``datetime`` objects, using the
desired time zone.
interface to make it return ``datetime`` objects using the desired time zone.

For your reference, in the following examples, epoch 1658167836758 is
``Mon, 18 Jul 2022 18:10:36 GMT``.
Expand Down
3 changes: 1 addition & 2 deletions docs/query.rst
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ converter function defined as ``lambda``, which assigns ``yes`` for boolean
=======================================

Based on the data type converter functionality, the driver offers a convenient
interface to make it return timezone-aware ``datetime`` objects, using the
desired time zone.
interface to make it return ``datetime`` objects using the desired time zone.

For your reference, in the following examples, epoch 1658167836758 is
``Mon, 18 Jul 2022 18:10:36 GMT``.
Expand Down
10 changes: 7 additions & 3 deletions src/crate/client/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,15 @@ def __init__(
- ``zoneinfo.ZoneInfo("Australia/Sydney")``
- ``+0530`` (UTC offset in string format)

The driver always returns timezone-"aware" `datetime` objects,
with their `tzinfo` attribute set.

When `time_zone` is `None`, the returned `datetime` objects are
"naive", without any `tzinfo`, converted using ``datetime.utcfromtimestamp(...)``.
using Coordinated Universal Time (UTC), because CrateDB is storing
timestamp values in this format.

When `time_zone` is given, the returned `datetime` objects are "aware",
with `tzinfo` set, converted using ``datetime.fromtimestamp(..., tz=...)``.
When `time_zone` is given, the timestamp values will be transparently
converted from UTC to use the given time zone.
""" # noqa: E501

self._converter = converter
Expand Down
6 changes: 3 additions & 3 deletions src/crate/client/converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
https://crate.io/docs/crate/reference/en/latest/interfaces/http.html#column-types
"""

import datetime as dt
import ipaddress
from copy import deepcopy
from datetime import datetime
from enum import Enum
from typing import Any, Callable, Dict, List, Optional, Union

Expand All @@ -45,13 +45,13 @@ def _to_ipaddress(
return ipaddress.ip_address(value)


def _to_datetime(value: Optional[float]) -> Optional[datetime]:
def _to_datetime(value: Optional[float]) -> Optional[dt.datetime]:
"""
https://docs.python.org/3/library/datetime.html
"""
if value is None:
return None
return datetime.utcfromtimestamp(value / 1e3)
return dt.datetime.fromtimestamp(value / 1e3, tz=dt.timezone.utc)
Copy link

Choose a reason for hiding this comment

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

Would it be faster to value * 1e-3?

Copy link
Member Author

Choose a reason for hiding this comment

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

idk.



def _to_default(value: Optional[Any]) -> Optional[Any]:
Expand Down
11 changes: 7 additions & 4 deletions src/crate/client/cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,12 +258,15 @@ def time_zone(self, tz):
- ``zoneinfo.ZoneInfo("Australia/Sydney")``
- ``+0530`` (UTC offset in string format)

The driver always returns timezone-"aware" `datetime` objects,
with their `tzinfo` attribute set.

When `time_zone` is `None`, the returned `datetime` objects are
"naive", without any `tzinfo`, converted using
`datetime.utcfromtimestamp(...)`.
using Coordinated Universal Time (UTC), because CrateDB is storing
timestamp values in this format.

When `time_zone` is given, the returned `datetime` objects are "aware",
with `tzinfo` set, converted by `datetime.fromtimestamp(..., tz=...)`.
When `time_zone` is given, the timestamp values will be transparently
converted from UTC to use the given time zone.
"""

# Do nothing when time zone is reset.
Expand Down
11 changes: 10 additions & 1 deletion tests/client/test_cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,16 @@ def test_execute_with_converter(self):
[
"foo",
IPv4Address("10.10.10.1"),
datetime.datetime(2022, 7, 18, 18, 10, 36, 758000),
datetime.datetime(
2022,
7,
18,
18,
10,
36,
758000,
tzinfo=datetime.timezone.utc,
kneth marked this conversation as resolved.
Show resolved Hide resolved
),
6,
],
[None, None, None, None],
Expand Down
Loading