shared: add UTCDateTime column type #179
Conversation
utnapischtim
commented
Dec 16, 2024
utnapischtim
commented
- with that it is possible to solve the datetime.utcnow DeprecationWarnings
utnapischtim
force-pushed
the
add-utcdatetime-type
branch
2 times, most recently
from
77dae3f to
6e8950b
Compare
invenio_db/shared.py
Outdated
| def process_bind_param(self, value, dialect): | ||
| """Process value storing into database.""" | ||
| if isinstance(value, datetime): | ||
| return value.replace(tzinfo=None) |
There was a problem hiding this comment.
minor: I think here we should also check that value.tzinfo in (None, timezone.utc), and only in that case proceed. I would even go as far as raising an exception if it's not the case, since implicitly dropping the timezone info (or converting from local to UTC) is the behavior we want to avoid.
Needs to be tested in a couple of complex modules first (e.g. invenio-rdm-records), but I have a feeling that it'll also help us to catch some datetime-related bugs :)
utnapischtim
force-pushed
the
add-utcdatetime-type
branch
3 times, most recently
from
622eae3 to
e1eb285
Compare
invenio_db/shared.py
Outdated
| ) | ||
| raise ValueError(msg) | ||
|
|
||
| return value.replace(tzinfo=None) |
There was a problem hiding this comment.
so it should be tzinfo=timezone.utc.
even if we are not storing utc (aka "+00:00") explicitly in the database, we assume it is utc and to be explicit what we retrieve from the database should be always explicitly utc (aka "+00:00")
* with that it is possible to solve the datetime.utcnow DeprecationWarnings
utnapischtim
force-pushed
the
add-utcdatetime-type
branch
from
e1eb285 to
0b9c920
Compare
2 participants
