dstandish commented on pull request #12910: URL: https://github.com/apache/airflow/pull/12910#issuecomment-741076236
update... **On impact to users** so the only time that `str(dt)` differs from `str(pendulum.instance(dt))` is when dt has not tz info. in our case, these attributes already always had tzinfo. so there is no difference in string representation as a result of this change. so i don't think there is any significant change in behavior that merits calling out in updating, apart from the mere fact that it's now pendulum instead of datetime **On the failing test** The cause of this failure is the way `get_previous_ti` works on `TaskInstance`. See here https://github.com/apache/airflow/blob/ff25bd6ffed53de651254f7c3e6d254e4191bfc7/airflow/models/taskinstance.py#L691-L696. Since we're running without a dag run, a new task instance is manufactured. But since this task instance has never been run, it has no start_date. So I added a check for presence of start_date. Instead of adding this check, I could have modified the test so that we test within a dag_run. In this case, the code would have tried to retrieve a previous dag run, and not finding one, would have returned None for `previous_ti`. Which is how it would generally behave in real life. But maybe there are reasons for supporting the no-dagrun case so I leave as is. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
