kaxil commented on a change in pull request #11358:
URL: https://github.com/apache/airflow/pull/11358#discussion_r502007626
##########
File path: airflow/jobs/scheduler_job.py
##########
@@ -1159,17 +1159,24 @@ def _change_state_for_tis_without_dagrun(
tis_to_change: List[TI] = query.with_for_update().all()
for ti in tis_to_change:
ti.set_state(new_state, session=session)
+ ti.duration = 0
tis_changed += 1
else:
subq = query.subquery()
+ current_time = timezone.utcnow()
tis_changed = session \
.query(models.TaskInstance) \
.filter(
models.TaskInstance.dag_id == subq.c.dag_id,
models.TaskInstance.task_id == subq.c.task_id,
models.TaskInstance.execution_date ==
subq.c.execution_date) \
- .update({models.TaskInstance.state: new_state},
synchronize_session=False)
+ .update({
+ models.TaskInstance.state: new_state,
+ models.TaskInstance.start_date: current_time,
+ models.TaskInstance.end_date: current_time,
+ models.TaskInstance.duration: 0,
Review comment:
>This doesn't feel right in the case of Sensing or Up-for-reschedule
tasks. Hmmm
That is true, we should only set end_date and duration when the new TI state
is either in one of "failed", "success" or "skipped". fixed in
https://github.com/apache/airflow/pull/11358/commits/3b0d977d413b38e6b394287a6cbc85a6bc25b414
>We can let the DB give us the time, no need to do a utcnow() this way.
I did that to have parity between MySQL, Sqlite and Postgres as for MySQL
and Sqlite `ti.set_state` is called which also uses `timezone.utcnow()`
----------------------------------------------------------------
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]
