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
