dstandish commented on a change in pull request #22184:
URL: https://github.com/apache/airflow/pull/22184#discussion_r828511240
##########
File path: airflow/dag_processing/processor.py
##########
@@ -412,20 +410,20 @@ def manage_slas(self, dag: DAG, session: Session = None)
-> None:
else:
while next_info.logical_date < ts:
next_info = dag.next_dagrun_info(next_info.data_interval,
restricted=False)
-
if next_info is None:
break
- if (ti.dag_id, ti.task_id, next_info.logical_date) in
recorded_slas_query:
+ next_run_id = DR.generate_run_id(DagRunType.SCHEDULED,
next_info.logical_date)
+ if (ti.dag_id, ti.task_id, next_run_id, ti.map_index) in
recorded_sla_misses:
Review comment:
> the current SLA behaviour of creating the SlaMiss record against the
next execution date is confusing (and likely wrong) so lets not confuse matters
more by changing it to by against a future run_id that may never exist.
i think that the notion of "next execution date" is maybe a little
misleading. it just means next, relative to the last one examined. so we're
in `manage_slas`. we start with a task. we look at the "last successful or
skipped TI". then we say, ok, let's look at the next run for that task --
relative to the last one that's done. and let's see if it's failed its SLA.
(e.g. cus it is still running). if so, let's create an SlaMiss for it. the
only time that the TI would not exist is when scheduler for whatever reason
isn't even creating the TI -- e.g. because it's catchup=True, or
max_active_tasks=1, or scheduler is having trouble. `next_run_id` would
probably be more accurately called `curr_run_id`.
wdyt? for now i'll rename that variable.
> (i.e. throw away most of this PR, sorry.)
no worries if that's how it goes. we should do what we need to do, and it
was a good exercise in any case. but i'm not convinced that it's not the right
change.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
