You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Ash Berlin-Taylor <as...@apache.org> on 2021/11/25 10:36:22 UTC
Lazy consensus: simplify XCom methods and remove broken "future" dates idea.
Hi All,
As part of finishing off AIP-39 (Timetables) where we re-keyed
TaskInstance from (dag_id, task_id, exeuction_date) to (dag_id,
task_id, run_id) I am looking at doing the same for XCom and ran into a
few snags.
Currently the docs for ti.xcom_push[1] say this:
> *execution_date* (/datetime/) – if provided, the XCom will not be
> visible until this date. This can be used, for example, to send a
> message to a task on a future date without it being immediately
> visible.
And when the XCom feature first landed in 2015 that was the case.
However in 2016 (released with 1.7.1) clear xcom when task starts #1180
<https://github.com/apache/airflow/pull/1180> landed which cleared the
XCom data for a task instance before starting it.
I've dug around a bit and I can't see how this "without it being
immediately visible" can actually work anymore, so I propose that in
2.3 we issue a warning when any value other than the logical_date of
the TI is passed to this value, and otherwise ignore this parameter.
Does anyone have any reason they'd like to keep this?
I'm asking for lazy consensus to remove this. If I don't hear anything
by Tuesday afternoon (Europe time) I'll go ahead with this plan. I will
likely open a draft PR before hand, but won't merge before then.
In a separate issue to the lazy consensus I seek, but related to XCom ,
I'm /thinking/ about deprecating XCom.get_many entirely and
include_prior_dates from ti.xcom_pull. Does anyone currently make use
of those?
Thanks,
Ash
[1]:
<https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/index.html#airflow.models.TaskInstance.xcom_push>
Re: Lazy consensus: simplify XCom methods and remove broken "future" dates idea.
Posted by Ash Berlin-Taylor <as...@apache.org>.
Draft PR here <https://github.com/apache/airflow/pull/19825> (I haven't
finished writing/adding all the tests needed for that)
On Thu, Nov 25 2021 at 14:11:38 +0100, Jarek Potiuk <ja...@potiuk.com>
wrote:
> I can even actively agree. not even lazy consent to that one Ash :).
> Sounds like a super-obscure feature that likely no-one knows exists
> (and it does not work anyway).
>
>
> On Thu, Nov 25, 2021 at 11:36 AM Ash Berlin-Taylor <ash@apache.org
> <ma...@apache.org>> wrote:
>>
>> Hi All,
>>
>> As part of finishing off AIP-39 (Timetables) where we re-keyed
>> TaskInstance from (dag_id, task_id, exeuction_date) to (dag_id,
>> task_id, run_id) I am looking at doing the same for XCom and ran
>> into a few snags.
>>
>> Currently the docs for ti.xcom_push[1] say this:
>>
>> execution_date (datetime) – if provided, the XCom will not be
>> visible until this date. This can be used, for example, to send a
>> message to a task on a future date without it being immediately
>> visible.
>>
>>
>> And when the XCom feature first landed in 2015 that was the case.
>> However in 2016 (released with 1.7.1) clear xcom when task starts
>> #1180 landed which cleared the XCom data for a task instance before
>> starting it.
>>
>> I've dug around a bit and I can't see how this "without it being
>> immediately visible" can actually work anymore, so I propose that in
>> 2.3 we issue a warning when any value other than the logical_date of
>> the TI is passed to this value, and otherwise ignore this parameter.
>>
>> Does anyone have any reason they'd like to keep this?
>>
>> I'm asking for lazy consensus to remove this. If I don't hear
>> anything by Tuesday afternoon (Europe time) I'll go ahead with this
>> plan. I will likely open a draft PR before hand, but won't merge
>> before then.
>>
>> In a separate issue to the lazy consensus I seek, but related to
>> XCom , I'm thinking about deprecating XCom.get_many entirely and
>> include_prior_dates from ti.xcom_pull. Does anyone currently make
>> use of those?
>>
>> Thanks,
>> Ash
>>
>> [1]:
>> <https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/index.html#airflow.models.TaskInstance.xcom_push>
Re: Lazy consensus: simplify XCom methods and remove broken "future" dates idea.
Posted by Jarek Potiuk <ja...@potiuk.com>.
I can even actively agree. not even lazy consent to that one Ash :).
Sounds like a super-obscure feature that likely no-one knows exists
(and it does not work anyway).
On Thu, Nov 25, 2021 at 11:36 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>
> Hi All,
>
> As part of finishing off AIP-39 (Timetables) where we re-keyed TaskInstance from (dag_id, task_id, exeuction_date) to (dag_id, task_id, run_id) I am looking at doing the same for XCom and ran into a few snags.
>
> Currently the docs for ti.xcom_push[1] say this:
>
> execution_date (datetime) – if provided, the XCom will not be visible until this date. This can be used, for example, to send a message to a task on a future date without it being immediately visible.
>
>
> And when the XCom feature first landed in 2015 that was the case. However in 2016 (released with 1.7.1) clear xcom when task starts #1180 landed which cleared the XCom data for a task instance before starting it.
>
> I've dug around a bit and I can't see how this "without it being immediately visible" can actually work anymore, so I propose that in 2.3 we issue a warning when any value other than the logical_date of the TI is passed to this value, and otherwise ignore this parameter.
>
> Does anyone have any reason they'd like to keep this?
>
> I'm asking for lazy consensus to remove this. If I don't hear anything by Tuesday afternoon (Europe time) I'll go ahead with this plan. I will likely open a draft PR before hand, but won't merge before then.
>
> In a separate issue to the lazy consensus I seek, but related to XCom , I'm thinking about deprecating XCom.get_many entirely and include_prior_dates from ti.xcom_pull. Does anyone currently make use of those?
>
> Thanks,
> Ash
>
> [1]: https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/index.html#airflow.models.TaskInstance.xcom_push
Re: Lazy consensus: simplify XCom methods and remove broken "future" dates idea.
Posted by Jarek Potiuk <ja...@potiuk.com>.
I can even actively agree. not even lazy consent to that one Ash :).
Sounds like a super-obscure feature that likely no-one knows exists
(and it does not work anyway).
On Thu, Nov 25, 2021 at 11:36 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>
> Hi All,
>
> As part of finishing off AIP-39 (Timetables) where we re-keyed TaskInstance from (dag_id, task_id, exeuction_date) to (dag_id, task_id, run_id) I am looking at doing the same for XCom and ran into a few snags.
>
> Currently the docs for ti.xcom_push[1] say this:
>
> execution_date (datetime) – if provided, the XCom will not be visible until this date. This can be used, for example, to send a message to a task on a future date without it being immediately visible.
>
>
> And when the XCom feature first landed in 2015 that was the case. However in 2016 (released with 1.7.1) clear xcom when task starts #1180 landed which cleared the XCom data for a task instance before starting it.
>
> I've dug around a bit and I can't see how this "without it being immediately visible" can actually work anymore, so I propose that in 2.3 we issue a warning when any value other than the logical_date of the TI is passed to this value, and otherwise ignore this parameter.
>
> Does anyone have any reason they'd like to keep this?
>
> I'm asking for lazy consensus to remove this. If I don't hear anything by Tuesday afternoon (Europe time) I'll go ahead with this plan. I will likely open a draft PR before hand, but won't merge before then.
>
> In a separate issue to the lazy consensus I seek, but related to XCom , I'm thinking about deprecating XCom.get_many entirely and include_prior_dates from ti.xcom_pull. Does anyone currently make use of those?
>
> Thanks,
> Ash
>
> [1]: https://airflow.apache.org/docs/apache-airflow/stable/_api/airflow/models/index.html#airflow.models.TaskInstance.xcom_push