dstandish edited a comment 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** This was failing in `tests/operators/test_python.py::TestPythonVirtualenvOperator` because the `start_date` on `previous_ti` is not populated in the test code. 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. I am not sure which approach would be more desirable, but in case there are reasons for supporting the no-dagrun case so I have left the test logic 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]
