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 qith 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