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:08:40 GMT
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/>

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