I wonder instead of getting the tasks again (which is surpringly not that trivial), it may make more sense to simply filter from downstream_tasks

to_skip = [t for t in downstream_tasks if self.task_id in t.upstream_task_ids]

when you say not that trivial isn't it just doing dag.task_dict[x] for x in t.upstream_task_ids which is not much different?

a different way to optimize would to not create the downstream_tasks list at all in the ignore_downstream_trigger_rules is False case. then we could just pass to skip x for x in context["task"].get_direct_relatives(upstream=False) if not x.is_teardown as a generator since skip method can handle that -- i.e. there's not really a need for selectively calling skip.

but stepping back, just to be clear, i'm not adding any new call to either get_flat_relatives or get_direct_relatives -- both calls were there and i don't increase their likelihood of calling or frequency. i just happened to move to a variable cus it was handy to debug something.

OK @uranusjr i went with that optimization... PTAL

note: i removed a debug log which emitted the full list of short circuited tasks so that way we could use generator but if you think that's no bueno LMK.

ok pushed the optimization / refactor a little further here. hopefully i don't break something. lemme know what you think. restored the debug logging, and now it will debug log the actual lists in either case

I think you can do execution_date = context["logical_date"], that one should be guaranteed to be a pendulum DateTime.

Logical date is typed as datetime too it seems.

And worse, it does not exist on DagRunPydantic, so mypy complains about that. i'm just keeping the insert

Ok actually... at least we can use cast("DateTime", dag_run.execution_date) which is a little cleaner than the assert i suppose

Pushing the boundary of the walrus I see…

But should line 274 be removed?

Pushing the boundary of the walrus I see…

yes, hehe

But should line 274 be removed?

No because on 274 it is a generator. only if we need to log.debug is there a need to "materialize" it in the form of a list, because then it needs to be logged and passed to skip, which makes another list out of it anyway.

So it's maybe a silly optimization but hey you started it 😛

But so generally it will just be generator here, passed to skip as generator. But if user enables debug logging, then when we log it we render it into a list at that log line, and then pass the same list down to skip.

Ah OK get_tasks_to_skip() is an iterator. Maybe move the line to below this block instead then?

Personally I’d maybe rename the function or to_skip to be clearer there’s an iterator involved (either iter_tasks_to_skip() or tasks_to_skip_iter)

Ah OK get_tasks_to_skip() is an iterator. Maybe move the line to below this block instead then?

don't follow -- what would that do?

just going to merge this... if you want to golf it further, you can tag me in review