-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add support for timezone aware datetime objects to SQLAlchemy dialect #92
Comments
With this small diff: diff --git a/src/crate/client/sqlalchemy/dialect.py b/src/crate/client/sqlalchemy/dialect.py
index 1f51acf..31cc650 100644
--- a/src/crate/client/sqlalchemy/dialect.py
+++ b/src/crate/client/sqlalchemy/dialect.py
@@ -20,7 +20,7 @@
# software solely pursuant to the terms of the relevant commercial agreement.
import logging
-from datetime import datetime, date
+from datetime import datetime, date, timezone
from sqlalchemy import types as sqltypes
from sqlalchemy.engine import default, reflection
@@ -121,7 +121,7 @@ class DateTime(sqltypes.DateTime):
if value is not None:
assert isinstance(value, datetime)
if value.tzinfo is not None:
- raise TimezoneUnawareException(DateTime.TZ_ERROR_MSG)
+ value = value.astimezone(timezone.utc).replace(tzinfo=None)
return value.strftime('%Y-%m-%dT%H:%M:%S.%fZ')
return value
return process The following example application would work: from datetime import datetime, timezone
from sqlalchemy import create_engine, Column
from sqlalchemy.schema import PrimaryKeyConstraint
from sqlalchemy.ext.declarative import DeclarativeMeta, declarative_base
from sqlalchemy.orm import scoped_session, sessionmaker
from sqlalchemy.types import DateTime, Float
Session = scoped_session(sessionmaker())
Base = declarative_base(metaclass=DeclarativeMeta)
class TestModel(Base):
__tablename__ = "t1"
__table_args__ = (PrimaryKeyConstraint("ts"), {"schema": "doc"})
ts = Column("ts", DateTime)
def main():
engine = create_engine(
"crate://",
connect_args={
"username": "crate",
"password": None,
"servers": "localhost:4200",
},
strategy="threadlocal",
)
Session.configure(bind=engine)
Base.metadata.bind = engine
session = Session()
session.execute("""
CREATE TABLE IF NOT EXISTS doc.t1 (
ts TIMESTAMP WITH TIME ZONE
)
""")
session.add(
TestModel(ts=datetime(2020, 6, 23, 12, 0, 0, tzinfo=timezone.utc))
)
session.commit()
if __name__ == "__main__":
main() |
I'm not sure if it is a good idea to discard information implicitly doing the serialization. Converting to UTC would keep the time information itself intact, but it would lose the timezone. Depending on the users use-case this may be required information and a user might assume that Seems to me that being strict about this is the better default. Maybe some opt-in flag cloud be provided. |
dt.strftime("%Y-%m-%dT%H:%M:%S.%f%z") This would not discard any information, but would add the TZ offset to the ISO string.
Agree, the comment above wasn't meant as a solution :)
|
You'd still loose part of the information eventually, even with |
Got you! But isn't it also somehow clear, that if the database stores the timestamp in UTC, that it will be converted and the offset is "lost"? Usually client applications take care that the timestamp from the server is converted into a "client local" timezone. |
Hi. Without reading about the specific details here, I just wanted to share a chance find. SQLAlchemy-Utc is a drop-in replacement of SQLAlchemy’s built-in DateTime type with With kind regards, |
Inserting a timezone aware
datetime
object using SQLAlchemy fails, even though, the object could be serialized into an ISO-format string that contains a timezone offset.https://github.com/crate/crate-python/blob/master/src/crate/client/sqlalchemy/dialect.py#L123-L125
Same applies for de-serializing.
It may even be nice to add support for
TIMESTAMP WITH(OUT) TIME ZONE AT TIME ZONE
fields.This issue came up when I tried to insert a datetime object from the
boto
client response that is timezone aware, raisingTimezoneUnawareException: Timezone aware datetime objects are not supported
The text was updated successfully, but these errors were encountered: