airflow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Driesprong, Fokko" <fo...@driesprong.frl>
Subject Re: [VOTE] Changes in import paths
Date Sat, 27 Jul 2019 07:07:30 GMT
I agree with all, this is a bit chaotic. For the sake of clarity, I would
suggest moving the Google Doc to the Wiki as an AIP. Create a structural
code improvement AIP and then and add a matrix of users/cases where
everybody can add his vote per case. This gives also the opportunity for
changing your vote on a specific case. WDYT?

Cheers, Fokko

Op vr 26 jul. 2019 om 22:28 schreef Chen Tong <cixuuz@gmail.com>:

> 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