jscheffl commented on code in PR #35868:
URL: https://github.com/apache/airflow/pull/35868#discussion_r1408255345
##########
airflow/migrations/versions/0027_1_10_0_add_time_zone_awareness.py:
##########
@@ -139,11 +139,11 @@ def upgrade():
type_=mysql.TIMESTAMP(fsp=6),
)
else:
- # sqlite and mssql datetime are fine as is. Therefore, not converting
Review Comment:
100% on your argument line - and I really had the same thoughts, just did
not write it like this.
Mainly I started removing migrations when I cleaned (future) dead code in
airflow/migrations/utils.py and then saw migration scripts failing because
common functions missing. Migration code is not 100% separated.
As we are shortly before the release and we need more days in
discussing/debating about migrations I'd also be OK to split the PR in two.
(with a minimum risk that I need to leave some migration dependency dead code
behind).
And still if somebody want to play with MSSQL there is the 2.7.3 release
available, from there still you could up-and downgrade. Leaving traces behind
if 2.8.0 does not support MSSQL anyway is just some leftover cleanup. And I
would not like to keep all MSSQL dependencies in the future (2.9++?) just
because in ancient history we supported MSSQL - rather clean it soon.
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
