airflow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tomasz Urbaszek <turbas...@apache.org>
Subject Re: [UPDATE] AIP-31 .output update
Date Wed, 17 Jun 2020 07:13:57 GMT
+1 (binding)

On Wed, Jun 17, 2020 at 3:39 AM 蒋晓峰 <thanosxnicholas@gmail.com> wrote:

> +1(not binding)
>
> On Wed, Jun 17, 2020 at 3:03 AM Gerard Casas Saez
> <gcasassaez@twitter.com.invalid> wrote:
>
> > 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