airflow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jarek Potiuk <Jarek.Pot...@polidea.com>
Subject Re: [VOTE] Changes in import paths
Date Fri, 26 Jul 2019 08:14:05 GMT
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/>

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