-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve ExternalTaskSensor execution date pattern #10883
Comments
Thanks for opening your first issue here! Be sure to follow the issue template! |
I'd like to work on this one! |
assigned! |
Current Work |
Hi @dmcochener am new here trying to help and learn |
It’s just the “execution_date” field that is not templating. |
Currently in the
ExternalTaskSensor
there are two ways two set the execution date of a DAG/task the sensor is waiting on:execution_delta
which simply adds/subtracts a time value to/from theexecution_date
of the sensor itselfexecution_date_fn
which is a function that takes in just theexecution_date
and returns a manipulated one.I believe that the correct way to go about handling the
execution_date
for theExternalTaskSensor
should be analogous to the way we handle it for other operators, such as theExternalTaskMarker
, or more relevantly, theTriggerDagRunOperator
, which is an operator usually used in conjunction with theExternalTaskSensor
.Specifically, we should remove the
execution_delta
parameter and replace it with a templatedexecution_date
parameter. This not only covers the existingexecution_delta
use case through e.g.{{ execution_date - macros.timedelta(days=1) }}
, but also provides more flexibility by enabling the benefits of templated parameters in general, such as pulling from xcoms, reading from DAG/operator params, using macros, etc. It also establishes the pattern that we see in the previously mentioned operators.The proposed solution is also sufficient enough to remove the
execution_date_fn
because we can register custom macros to manipulate the execution date the same way as anexecution_date_fn
would. But I do realize we may want to keep it for backwards compatibility reasons.First time contributor here so happy to implement/help with this to get some airflow OSS chops.
The text was updated successfully, but these errors were encountered: