airflow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chen Tong <cix...@gmail.com>
Subject Re: [VOTE] Changes in import paths
Date Fri, 26 Jul 2019 20:28:02 GMT
I agree with Ash. It is clear to vote on each case rather than all
together.

On Fri, Jul 26, 2019 at 3:54 PM Ash Berlin-Taylor <ash@apache.org> wrote:

> A number of proposals overlap or add on to each other, so I think (?) a
> single vote makes sense.
>
> To be clear, my vote is -1 until it's clear exactly what we are voting on.
>
> On 26 July 2019 20:39:56 BST, Kaxil Naik <kaxilnaik@gmail.com> wrote:
> >I agree with Ash and instead of voting on all 7 Cases together, how
> >about
> >separate vote emails for all the cases? Each email can have the
> >description
> >copied over from the doc.
> >
> >It would probably be a bit easier to track a decision in the future as
> >well.
> >
> >What do you guys think? @Jarek Potiuk <Jarek.Potiuk@polidea.com>
> >@Driesprong,
> >Fokko <fokko@driesprong.frl>
> >
> >On Sat, Jul 27, 2019 at 1:03 AM Kaxil Naik <kaxilnaik@gmail.com> wrote:
> >
> >> *Case #1* airflow.contrib.{resources} : *Solution A *
> >>
> >> *Case #2* git *_{operator/sensor}{/s}.py : *Solution A *
> >>
> >> *Case #3* {aws/azure/gcp}_* : *Solution C*
> >>
> >> *Case #4* Separate namespace for resources :
> >> *Solution A*
> >>
> >> *Case #5* *Operator : *Solution A*
> >>
> >> *Case #7* : *Solution A*
> >>
> >> On Fri, Jul 26, 2019 at 11:09 PM Daniel Standish
> ><dpstandish@gmail.com>
> >> wrote:
> >>
> >>> I have tried to add some clarification to Jarek's summary, but I am
> >a
> >>> little fuzzy on exact proposal for case 3 so hopefully Jarek can
> >further
> >>> clarify.
> >>>
> >>> According to my reading, in this motion cases 4,5,6 are all either
> >>> proposal
> >>> *rejections* or otherwise have no effect, so attention can be
> >focused on
> >>> cases 1,2,3 and 7.
> >>>
> >>> *Motion is to adopt the following:*
> >>>
> >>> Case 1 - Solution A - Get rid of contrib
> >>> All objects will be moved out of contrib folder and into respective
> >>> non-contrib folder.
> >>>
> >>> Case 2 - Solution A - Remove object type suffix from modules
> >>> Example:
> >>> Airflow.operators.foo_operator.FooOperator becomes
> >>> airflow.operators.foo.FooOperator
> >>> Example:
> >>> Airflow.hooks.foo_hook.FooOperator becomes airflow.hooks.foo.FooHook
> >>>
> >>> Case 3 - conventions for cloud provider etc prefixes
> >>> *I am not sure exactly what the proposal is.*
> >>> Example case is airflow/contrib/operators/gcp_bigtable_operator.py
> >>> Since motion adopts case 1 solution A, we can assume it is now named
> >like
> >>> so: airflow/operators/gcp_bigtable_operator.py
> >>> So what is proposed for this case?
> >>>
> >>> Case 4 - Solution B - *no change*
> >>> This proposal was about namespaces but the motion is to reject the
> >>> proposal.
> >>>
> >>> Case 5 - Solution B - *no change*
> >>> The proposal was to remove class type suffix e.g. remove the
> >Operator in
> >>> FooOperator, but the motion is to reject this proposal -- i.e.
> >motion is
> >>> no
> >>> change on this case, i.e. we resolve to keep "Operator" (and
> >"Sensor")
> >>> suffixes on class names
> >>>
> >>> Case 6 - *No change*
> >>> This case concerns object naming inconsistencies, but motion does
> >not
> >>> enact
> >>> any specific proposal.
> >>>
> >>> Case 7 - Solution A - use warnings.warn template for deprecation
> >warnings
> >>> (instead of zope)
> >>> Example:
> >>>
> >>> # in old_package/old_mod.py
> >>>
> >>> import warnings
> >>> from new_package.new_mod import *
> >>> warnings.warn("old_package.old_mod has moved to new_package.new_mod.
> >>> Import
> >>> of "
> >>> "old_package.old_mod will become unsupported in version 2",
> >>> DeprecationWarning, 2)
> >>>
> >>> Reference implementation here:
> >>>
> >>>
> >
> https://github.com/mik-laj/airflow-deprecation-sample/tree/master/solution1
> >>>
> >>>
> >>> FWIW +1 (non-binding) on 1,2,7 -- unsure on 3.
> >>>
> >>> I am very happy that this motion now gets rid of contrib.  Thank you
> >>> Sergio
> >>> for turning the tide with your effective argumentation ;)
> >>>
> >>>
> >>>
> >>>
> >>>
> >>> On Fri, Jul 26, 2019 at 3:16 AM Ash Berlin-Taylor <ash@apache.org>
> >wrote:
> >>>
> >>> > Could someone summarise what the proposed name changes are, here,
> >in
> >>> this
> >>> > thread? Pointing at a google doc and only giving partial examples
> >is 1)
> >>> a
> >>> > bit difficult to quickly work out what we are voting on, and 2)
> >using a
> >>> > google doc isn't great for a longer term record.
> >>> >
> >>> > Thanks,
> >>> > Ash
> >>> >
> >>> > > On 26 Jul 2019, at 09:14, Jarek Potiuk
> ><Jarek.Potiuk@polidea.com>
> >>> wrote:
> >>> > >
> >>> > > I would like to recast the vote. Let's start from 0 :) . I would
> >like
> >>> to
> >>> > > keep the July 30th 6pm CEST date, and at least three +1
> >(binding)
> >>> votes
> >>> > are
> >>> > > needed to pass. I marked in red the modifications to the
> >previous
> >>> > proposal.
> >>> > >
> >>> > > Here is the proposal (details here
> >>> > > <
> >>> >
> >>>
> >
> https://docs.google.com/document/d/1F8zve5S78DXcjpPttW89HnqT0M0iKjT6fo9jX57Ef6A/edit#heading=h.j4jc3i70qszo
> >>> > >
> >>> > > )
> >>> > >
> >>> > >   - *Case 1: A: Get rid of contrib,*
> >>> > >   - *Case 2: A: Airflow.operators.foo_operator.FooOperator could
> >>> > >   become airflow.operators.foo.FooOperator*
> >>> > >   - *Case 3: C:
> >>> > >
> >airflow.contrib.operators.gcp_bigtable_operator.BigTableOperator
> >>> could
> >>> > >   become airflow.gcp.operators.bigtable.BigTableOperator*
> >>> > >   - *Case 4: B: no namespace introduction*
> >>> > >   - *Case 5: B: Keep "Operator" (and "Sensor") suffixes on class
> >>> names*
> >>> > >   - *Case 6: We will treat isolated cases on case-by-case (and
> >my team
> >>> > can
> >>> > >   do the job of GCP-related operators)*
> >>> > >   - *Case 7: Native python solution (with better IDE
> >integration)*
> >>> > >
> >>> > > This is my binding (+1) vote.
> >>> > >
> >>> > >
> >>> > >
> >>> > > On Fri, Jul 26, 2019 at 10:08 AM Jarek Potiuk <
> >>> Jarek.Potiuk@polidea.com>
> >>> > > wrote:
> >>> > >
> >>> > >> Hey Felix,
> >>> > >>
> >>> > >> For 7* -> I am in favour of trying the native one as well
(I
> >use IDE
> >>> > quite
> >>> > >> often ;) )
> >>> > >>
> >>> > >> J.
> >>> > >>
> >>> > >> On Wed, Jul 24, 2019 at 9:34 AM Driesprong, Fokko
> >>> <fokko@driesprong.frl
> >>> > >
> >>> > >> wrote:
> >>> > >>
> >>> > >>> Thanks Kamil for putting the document together. I wasn't
able
> >to
> >>> > respond
> >>> > >>> earlier since I was giving Airflow workshops throughout
Europe
> >:-)
> >>> > >>>
> >>> > >>> *Case 1: *Solution A → Remove everything from contrib
into a
> >single
> >>> > >>> package.
> >>> > >>>
> >>> > >>> To make it easier to the end-user, my preference would
be to
> >get
> >>> rid of
> >>> > >>> the
> >>> > >>> contrib package altogether. What's in contrib or not is
a
> >tricky
> >>> > question,
> >>> > >>> I think this is already reflected in the document since
> >"maturity"
> >>> is
> >>> > in
> >>> > >>> quotes. What would be the moment to transfer the operator
to
> >the
> >>> core
> >>> > or
> >>> > >>> vice versa? I'm afraid that renaming contrib to incubating
> >will
> >>> confuse
> >>> > >>> the
> >>> > >>> user even more. For people who aren't as known with Airflow
it
> >isn't
> >>> > that
> >>> > >>> transparent which operator lives where, especially if
you
> >don't use
> >>> an
> >>> > IDE
> >>> > >>> such as Ash ;) Besides that I don't think it is worth
changing
> >the
> >>> > public
> >>> > >>> API just for a different name.
> >>> > >>>
> >>> > >>
> >>> > >> Ok. I am convinced. I will re-cast the vote with this. It
will
> >make
> >>> > easier
> >>> > >> as we will not have to define other rules as those that we
> >already
> >>> have.
> >>> > >>
> >>> > >>
> >>> > >>> *Case 2:* Slight preference to Solution B (let's say a
+0)
> >>> > >>>
> >>> > >>> I like to search on Github with t, and then search for
> >BashOperator.
> >>> > After
> >>> > >>> the change, you should search for Operator/Bash.
> >>> > >>>
> >>> > >>> Jarek, I'm confused with your vote on this, from your
mail you
> >>> state: -
> >>> > >>> *Case 2: B: Airflow.operators.foo_operator.FooOperator
could
> >become
> >>> > >>> airflow.operators.foo.FooOperator*. But the B solution
is
> >keeping it
> >>> > the
> >>> > >>> same, so that would mean - *Case 2: B:
> >>> > >>> Airflow.operators.foo_operator.FooOperator *would stay
> >>> > >>> airflow.operators.foo_operator*.FooOperator*
> >>> > >>>
> >>> > >>> My mistake. It was of course A. I think there is a little
> >confusion
> >>> > here.
> >>> > >> This case is not for changing BashOperator into Bash but for
> >changing
> >>> > the
> >>> > >> *module* name not the *class* name. So
> >bash_operator.BashOperator ->
> >>> > >> bash.BashOperator :) . you will still search for BashOperator
> >in this
> >>> > case.
> >>> > >>
> >>> > >>
> >>> > >>> *Case 3:* I'm leaning towards B
> >>> > >>>
> >>> > >>> Mostly because it isn't as trivial to decide when it gets
its
> >own
> >>> > package
> >>> > >>> or not. Besides the cloud providers, there are companies
like
> >>> > Databricks
> >>> > >>> and Qubole which might also end up with their own package
> >because
> >>> their
> >>> > >>> integration is getting richer over time. Moving this is
a bit
> >>> painful
> >>> > >>> because we need to deprecate the old path and move everything
> >which
> >>> > >>> introduces noise into version control etc.
> >>> > >>
> >>> > >>
> >>> > >> I'd say, we should go for C. As I proposed. It's not as
> >difficult as
> >>> it
> >>> > >> seems to group operators in packages and it has numerous
> >advantages.
> >>> The
> >>> > >> nice thing about deprecations - we can do them in the
> >__init__.py of
> >>> the
> >>> > >> packages which causes the noise fairly small (Git will actually
> >>> realise
> >>> > >> that the file has been moved and you can follow that. Maybe
not
> >as
> >>> super
> >>> > >> easy to merge changes later to 1.10.4  but it's not a rocket
> >science
> >>> > either.
> >>> > >>
> >>> > >>
> >>> > >>> *Case 7:* Python native solution
> >>> > >>>
> >>> > >>
> >>> > >> Yes.
> >>> > >>
> >>> > >>
> >>> > >>>
> >>> > >>> ---
> >>> > >>>
> >>> > >>> Apart from all, great work!
> >>> > >>>
> >>> > >>> Cheers, Fokko
> >>> > >>>
> >>> > >>>
> >>> > >>> Op di 23 jul. 2019 om 21:27 schreef Felix Uellendall
> >>> > >>> <feluelle@pm.me.invalid
> >>> > >>>> :
> >>> > >>>
> >>> > >>>> I have no strong "No" against any proposed change
of these
> >cases.
> >>> So I
> >>> > >>> go
> >>> > >>>> with +1 (non-binding).
> >>> > >>>>
> >>> > >>>> P.S. Thanks Jarek for bringing this up again and your
intense
> >work
> >>> > >>> towards
> >>> > >>>> airflow currently :) and thanks to Kamil for even
creating
> >this
> >>> > >>> document. I
> >>> > >>>> like how the code is getting more and more consistent
and
> >clean :)
> >>> > >>>>
> >>> > >>>> Kind regards,
> >>> > >>>> Felix
> >>> > >>>>
> >>> > >>>> Sent from ProtonMail mobile
> >>> > >>>>
> >>> > >>>> -------- Original Message --------
> >>> > >>>> On Jul 23, 2019, 18:34, Jarek Potiuk wrote:
> >>> > >>>>
> >>> > >>>>> Hello everyone,
> >>> > >>>>>
> >>> > >>>>> This email is calling a vote on the changes in
import paths.
> >It's
> >>> > been
> >>> > >>>>> discussed in
> >>> > >>>>>
> >>> > >>>>
> >>> > >>>
> >>> >
> >>>
> >
> https://lists.apache.org/thread.html/4e648d9421c792d4537f5ac66f1a16dce468f816fc5221a9f9db9433@%3Cdev.airflow.apache.org%3E
> >>> > >>>>>
> >>> > >>>>> The vote will last for at least 1 week (July 30th
6pm CEST),
> >and
> >>> at
> >>> > >>> least
> >>> > >>>>> three +1 (binding) votes have been cast.
> >>> > >>>>>
> >>> > >>>>> The proposal to vote is based on the document
from Kamil
> >>> > >>>>> <
> >>> > >>>>
> >>> > >>>
> >>> >
> >>>
> >
> https://docs.google.com/document/d/1F8zve5S78DXcjpPttW89HnqT0M0iKjT6fo9jX57Ef6A/edit#
> >>> > >>>>>
> >>> > >>>>>
> >>> > >>>>> The proposed solution is:
> >>> > >>>>>
> >>> > >>>>> - *Case 1: B: Contrib vs not: we move all that
are "well"
> >tested
> >>> and
> >>> > >>>>> rename contrib to "incubating" or similar.*
> >>> > >>>>> - *Case 2: B: Airflow.operators.foo_operator.FooOperator
> >could
> >>> > >>>>> become airflow.operators.foo.FooOperator*
> >>> > >>>>> - *Case 3: C:
> >>> > >>>>>
> >airflow.contrib.operators.gcp_bigtable_operator.BigTableOperator
> >>> > could
> >>> > >>>>> become airflow.gcp.operators.bigtable.BigTableOperator*
> >>> > >>>>> - *Case 4: B: no namespace introduction*
> >>> > >>>>> - *Case 5: B: Keep "Operator" (and "Sensor") suffixes
on
> >class
> >>> names*
> >>> > >>>>> - *Case 6: We will treat isolated cases on case-by-case
(and
> >my
> >>> team
> >>> > >>> can
> >>> > >>>>> do the job of GCP-related operators)*
> >>> > >>>>>
> >>> > >>>>> This is my (binding) +1 vote.
> >>> > >>>>>
> >>> > >>>>> Best regards,
> >>> > >>>>>
> >>> > >>>>> J.
> >>> > >>>>>
> >>> > >>>>> --
> >>> > >>>>>
> >>> > >>>>> Jarek Potiuk
> >>> > >>>>> Polidea <https://www.polidea.com/> | Principal
Software
> >Engineer
> >>> > >>>>>
> >>> > >>>>> M: [+48 660 796 129](tel:+48660796129)
> >>> > >>> <[+48660796129](tel:+48660796129)>
> >>> > >>>>> [image: Polidea] <https://www.polidea.com/>
> >>> > >>>
> >>> > >>
> >>> > >>
> >>> > >> --
> >>> > >>
> >>> > >> Jarek Potiuk
> >>> > >> Polidea <https://www.polidea.com/> | Principal Software
> >Engineer
> >>> > >>
> >>> > >> M: +48 660 796 129 <+48660796129>
> >>> > >> [image: Polidea] <https://www.polidea.com/>
> >>> > >>
> >>> > >>
> >>> > >
> >>> > > --
> >>> > >
> >>> > > Jarek Potiuk
> >>> > > Polidea <https://www.polidea.com/> | Principal Software
Engineer
> >>> > >
> >>> > > M: +48 660 796 129 <+48660796129>
> >>> > > [image: Polidea] <https://www.polidea.com/>
> >>> >
> >>> >
> >>>
> >>
> >>
> >> --
> >> *Kaxil Naik*
> >> *Big Data Consultant | DevOps Data Engineer*
> >> *Certified *Google Cloud Data Engineer | *Certified* Apache Spark &
> >Neo4j
> >> Developer
> >> *LinkedIn*: https://www.linkedin.com/in/kaxil
> >>
> >
> >
> >--
> >*Kaxil Naik*
> >*Big Data Consultant | DevOps Data Engineer*
> >*Certified *Google Cloud Data Engineer | *Certified* Apache Spark &
> >Neo4j
> >Developer
> >*LinkedIn*: https://www.linkedin.com/in/kaxil
>

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