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 Re: [UPDATE] AIP-31 .output update
Date Thu, 18 Jun 2020 21:38:52 GMT
Closing this vote after 48h as announced.

+5 (bindings) +2 (non-binding)
No negative votes or objections.

We will proceed with the current updated AIP. Thank you everyone!

Gerard Casas Saez
Twitter | Cortex | @casassaez
On Jun 18, 2020, 11:02 AM -0600, Jarek Potiuk <Jarek.Potiuk@polidea.com>, wrote:
> +1 (binding). I like where the functional DAG is going. :)
>
> On Thu, Jun 18, 2020 at 6:41 PM Xinbin Huang <bin.huangxb@gmail.com> wrote:
>
> > +1 (non-binding)
> >
> > On Thu, Jun 18, 2020 at 9:36 AM Kamil Breguła <kamil.bregula@polidea.com>
> > wrote:
> >
> > > +1 (binding)
> > >
> > > On Thu, Jun 18, 2020, 18:33 Dan Davydov <ddavydov@twitter.com.invalid>
> > > wrote:
> > >
> > > > +1 (binding)
> > > >
> > > > On Thu, Jun 18, 2020 at 12:32 PM Daniel Imberman <
> > > > daniel.imberman@gmail.com>
> > > > wrote:
> > > >
> > > > > +1 (binding)
> > > > >
> > > > > via Newton Mail [
> > > > >
> > > >
> > >
> > https://cloudmagic.com/k/d/mailapp?ct=dx&cv=10.0.50&pv=10.14.6&source=email_footer_2
> > > > > ]
> > > > > On Wed, Jun 17, 2020 at 12:13 AM, Tomasz Urbaszek <
> > > turbaszek@apache.org>
> > > > > wrote:
> > > > > +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
> > > > > > >
> > > > > >
> > > >
> > >
> >
>
>
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>

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