ephraimbuddy commented on code in PR #24044:
URL: https://github.com/apache/airflow/pull/24044#discussion_r889961614
##########
airflow/utils/sqlalchemy.py:
##########
@@ -42,21 +43,25 @@
class UtcDateTime(TypeDecorator):
"""
- Almost equivalent to :class:`~sqlalchemy.types.DateTime` with
+ Almost equivalent to :class:`~sqlalchemy.types.TIMESTAMP` with
``timezone=True`` option, but it differs from that by:
- Never silently take naive :class:`~datetime.datetime`, instead it
always raise :exc:`ValueError` unless time zone aware value.
- :class:`~datetime.datetime` value's :attr:`~datetime.datetime.tzinfo`
is always converted to UTC.
- - Unlike SQLAlchemy's built-in :class:`~sqlalchemy.types.DateTime`,
+ - Unlike SQLAlchemy's built-in :class:`~sqlalchemy.types.TIMESTAMP`,
it never return naive :class:`~datetime.datetime`, but time zone
aware value, even with SQLite or MySQL.
- - Always returns DateTime in UTC
+ - Always returns TIMESTAMP in UTC
"""
- impl = DateTime(timezone=True)
+ impl = (
+ TIMESTAMP(timezone=True)
+ .with_variant(mssql.DATETIME2(precision=6), 'mssql')
+ .with_variant(mysql.TIMESTAMP(fsp=6), 'mysql')
+ )
Review Comment:
`TIMESTAMP` is a subclass of `DateTime` with fractional seconds
support(https://docs.sqlalchemy.org/en/14/core/type_basics.html#sqlalchemy.types.DateTime).
We use it in migration file(`db_types.TIMESTAMP`). This change makes it the
same with DB such that the default is `TIMESTAMP` with the different
variations. This will help in other DBs not supported yet to be in uniform as
we would use `db_types.TIMESTAMP` for migrations involving datetime+timezone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
