airflow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gerard Casas Saez <gcasass...@twitter.com.INVALID>
Subject [UPDATE] AIP-31 .output update
Date Tue, 16 Jun 2020 19:03:06 GMT
Hi everyone,

Sending an email here to consolidate an update to the AIP-31 that has happened while we have
been implementing this.

When AIP-31 was proposed, the proposal mentioned that all operators would have a __call__ method
which would be used to define DAG in a functional manner. While implementing this with the
SIG Functional Ops team, we decided to change that requirement and replace it with a better
integration with templated_fields and a new property in BaseOperator called output. The
idea is that (see AIP DAG example for variable reference) get_ip() == get_ip.output.

I updated the AIP-31 doc to reflect those changes. Also took the chance to clean the doc a
bit to reword some smaller stuff, in order to make it easier for consumption by people outside
of this group in the future.
	Update diff: https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=148638736&selectedPageVersions=12&selectedPageVersions=11
	Updated AIP-31: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=148638736

Regarding the output property change, you can follow this discussion in the current Airflow
PR between me and Ash, to understand better the motivations behind it: https://github.com/apache/airflow/pull/8962/files#r441004317.

A short summary:
	The main reason for this change was that sometimes templated args in operators are required
on initialization (example: subject on EmailOperator). This meant that you needed to add
a placeholder value on initialization, and then specify the XComArg that needed to be used
instead in the __call__ method. This seemed a bit confusing, so when implementing we decided
to rollback the __call__ addition to BaseOperator.

	This made it difficult to get XComArg from an operator as the initializer returns an instance
of the operator instead of an XComArg compared to the proposed __call__ method. For that,
we proposed to add a new property output to BaseOperator that would generate an XComArg
for the operator with return_value as the default key.

An example of how different the DAG looks between the previous AIP and the new update can
be found here: https://github.com/apache/airflow/pull/8962/files#r441013469

With that said, I will follow Ash recommendation and try to get a lazy consensus on this change:
	Allow 48h for committers and developers to comment/bring up objections on the change.

If no objection is raised by Thursday June 18th at 1PM MST, we will move forward with the
change  and merge https://github.com/apache/airflow/pull/8962 PR.

Thanks everyone for all the work on this change! 🎉

Gerard Casas Saez
Twitter | Cortex | @casassaez

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message