airflow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ash Berlin-Taylor <...@apache.org>
Subject Re: [2.0 spring cleaning] Changes in import paths
Date Wed, 05 Jun 2019 14:30:29 GMT
> Case 7: Zope deprecation or not
> *B*: I prefer to use Kamil's in-house hack than zope, but I feel we could
> improve it slightly to remove a bit of boilerplate - it looks like the
> warning.warn() message should be very similar in all deprecation cases and
> possibly we can define it in one place (providing that DepracationWarning
> will continue to work well with IDEs)

At this point wouldn't or home-rolled solution be very similar to zope.deprecation (which
is already a dep of Airflow)?

Also are we sure that zope.deprecation isn't supported by an IDE? It still issues a warning.warn.
(The example docs in zope.deprecation show messing about with sys.modules but that isn't needed.)


https://gist.github.com/ashb/327a79d7c6c1f5a2344cd1812a8ee92d shows a formatted example:

```
#Place this as airflow/foo.py
import zope.deprecation
zope.deprecation.moved('airflow.settings', 'version 2')
```

And then when you import that module you get:

```
/Users/ash/.virtualenvs/airflow/bin/ipython:1: DeprecationWarning: airflow.foo has moved to
airflow.settings. Import of airflow.foo will become unsupported in version 2
  #!/Users/ash/.virtualenvs/airflow/bin/python3.7

```

That should work fine with IDE?


> On 5 Jun 2019, at 14:56, Jarek Potiuk <Jarek.Potiuk@polidea.com> wrote:
> 
> I think it's very important to discuss this and choose the right approach
> and follow up quickly. This is very important to introduce some sanity in
> the structure of 2.0.0.
> I propose that anyone with a strong opinion voices his or her concerns here
> and then maybe a week from now we can get to a consensus and vote on the
> direction we take?
> 
> My preferences (but I can be convinced to change some of my choices) are:
> 
> Case 1: (contrib vs not)
> *B* (move all that are "well" tested): I think having "incubator" kind of
> approach for not super tested/documented but still usable code is useful.
> Though we would have to establish some objective criteria and describe a
> "growing from incubation" process.
> 
> Case 2 airflow.operators.foo_operator.FooOperator could become
> *A:*  airflow.operators.foo.FooOperator
> 
> Case 3: airflow.contrib.operators.gcp_bigtable_operator.BigTableOperator
> could become
> *C*: airflow.integration.gcp.operators.bigtable.BigTableOperator
> 
> Case 4: (separate packages/namesapces)
> *B:* no namespaces - i think we are very far from modularisation to happen
> (and how) and it might get never implemented so there are no real benefits
> I see now.
> 
> Case 5: "Operator" suffix on class names
> *B*: I prefer to keep B as this will be much easier when you search for
> class using your IDE.
> 
> Case 6: "Isolated" renames
> *Rename*: We should align names of those operators.
> 
> Case 7: Zope deprecation or not
> *B*: I prefer to use Kamil's in-house hack than zope, but I feel we could
> improve it slightly to remove a bit of boilerplate - it looks like the
> warning.warn() message should be very similar in all deprecation cases and
> possibly we can define it in one place (providing that DepracationWarning
> will continue to work well with IDEs)
> 
> J,
> 
> On Tue, Jun 4, 2019 at 2:11 PM Kamil Breguła <kamil.bregula@polidea.com>
> wrote:
> 
>> I created AIP-21
>> 
>> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
>> This is the same document as above, I just deleted my opinions (summary
>> sections) and work plan.
>> 
>> 
>> On Fri, Apr 19, 2019 at 2:18 PM Jarek Potiuk <Jarek.Potiuk@polidea.com>
>> wrote:
>> 
>>> Thanks Kamil! Excellent analysis!
>>> 
>>> My preferences:
>>> 
>>> Case 1: (contrib vs not)
>>> B (move all that are "well" tested): I think having "incubator" kind of
>>> approach for not super tested/documented but still usable code is useful.
>>> Though we would have to establish some objective criteria and describe a
>>> "growing from incubation" process.
>>> 
>>> Case 2 airflow.operators.foo_operator.FooOperator could become
>>> A:  airflow.operators.foo.FooOperator
>>> 
>>> Case 3: airflow.contrib.operators.gcp_bigtable_operator.BigTableOperator
>>> could become
>>> C: airflow.integration.gcp.operators.bigtable.BigTableOperator
>>> 
>>> Case 4: (separate packages/namesapces)
>>> B: no namespaces - i think we are very far from modularisation to happen
>>> (and how) and it might get never implemented so there are no real
>> benefits
>>> I see now.
>>> 
>>> Case 5: "Operator" suffix on class names
>>> B: I prefer to keep B as this will be much easier when you search for
>> class
>>> using your IDE.
>>> 
>>> Case 7: Zope deprecation or not
>>> B: I prefer to use Kamil's in-house hack than zope, but I feel we could
>>> improve it slightly to remove a bit of boilerplate - it looks like the
>>> warning.warn() message should be very similar in all deprecation cases
>> and
>>> possibly we can define it in one place (providing that DepracationWarning
>>> will continue to work well with IDEs)
>>> 
>>> 
>>> 
>>> 
>>> On Fri, Apr 19, 2019 at 12:36 AM Kamil Breguła <
>> kamil.bregula@polidea.com>
>>> wrote:
>>> 
>>>> Inline comment
>>>> 
>>>> On Fri, Apr 19, 2019 at 12:23 AM Arthur Wiedmer <
>>> arthur.wiedmer@gmail.com>
>>>> wrote:
>>>> 
>>>>> 
>>>>>>>>> Case #5 Are we talking about the class name or the file
name?
>>> The
>>>>>> class
>>>>>>>>> name is fine to me, but we could remove _operator from
the
>> file
>>>>> name.
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> Case #2 describes the change of file names
>>>>>>>> Case #5 describes the change of class names
>>>>>>>> 
>>>>>>>> I added examples to two cases to better describe the changes.
>>>>>>>> 
>>>>>>> 
>>>>>>> Thank you. I guess I see it for file names, but I am wondering
>>> about
>>>>> the
>>>>>>> operators and name collisions.
>>>>>>> Say I need both the HiveOperator and I inline a custom operator
>> for
>>>>>> which I
>>>>>>> need a hive hook.
>>>>>>> # Would you recommmend the following? Or does case #5 only apply
>> to
>>>>>>> Operators?
>>>>>>> from airflow.operators.hive import Hive as HiveOperator
>>>>>>> from airfow.hooks.hive import Hive as HiveHook
>>>>>>> 
>>>>>>> I was just thinking it might be nice to be able to import what
I
>>> need
>>>>>>> without renaming and not worry too much about names shadowing
>>> others.
>>>>>>> Especially if I am new-ish to Apache Airflow.
>>>>>>> 
>>>>>>> 
>>>>>> I think that the operator should describe the behavior(verb), hook
>>>> should
>>>>>> describe a service(noun). These are other parts of speech.
>>>>>> In my opinion, HiveOperator should be named HiveExecuteQuery. Hook
>>> will
>>>>> be
>>>>>> called Hive. Then there will be no name conflicts.
>>>>>> 
>>>>> 
>>>>> But we will still provide the backwards compatibility for a while
>> with
>>>>> aliases to the old names, correct?
>>>>> 
>>>> 
>>>> Yes. It is possible to provide backward compatibility also in this
>> case.
>>>> 
>>>> 
>>>> --
>>>> 
>>>> Kamil Breguła
>>>> Polidea <https://www.polidea.com/> | Software Engineer
>>>> 
>>>> M: +48 505 458 451 <+48505458451>
>>>> E: kamil.bregula@polidea.com
>>>> [image: Polidea] <https://www.polidea.com/>
>>>> 
>>>> We create human & business stories through technology.
>>>> Check out our projects! <https://www.polidea.com/our-work>
>>>> [image: Github] <https://github.com/Polidea> [image: Facebook]
>>>> <https://www.facebook.com/Polidea.Software> [image: Twitter]
>>>> <https://twitter.com/polidea> [image: Linkedin]
>>>> <https://www.linkedin.com/company/polidea> [image: Instagram]
>>>> <https://instagram.com/polidea> [image: Behance]
>>>> <https://www.behance.net/polidea>
>>>> 
>>> 
>>> 
>>> --
>>> 
>>> Jarek Potiuk
>>> Polidea <https://www.polidea.com/> | Principal Software Engineer
>>> 
>>> M: +48 660 796 129 <+48660796129>
>>> E: jarek.potiuk@polidea.com
>>> 
>> 
>> 
>> --
>> 
>> Kamil Breguła
>> Polidea <https://www.polidea.com/> | Software Engineer
>> 
>> M: +48 505 458 451 <+48505458451>
>> E: kamil.bregula@polidea.com
>> [image: Polidea] <https://www.polidea.com/>
>> 
>> We create human & business stories through technology.
>> Check out our projects! <https://www.polidea.com/our-work>
>> [image: Github] <https://github.com/Polidea> [image: Facebook]
>> <https://www.facebook.com/Polidea.Software> [image: Twitter]
>> <https://twitter.com/polidea> [image: Linkedin]
>> <https://www.linkedin.com/company/polidea> [image: Instagram]
>> <https://instagram.com/polidea> [image: Behance]
>> <https://www.behance.net/polidea>
>> 
> 
> 
> -- 
> 
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
> 
> M: +48 660 796 129 <+48660796129>
> E: jarek.potiuk@polidea.com


Mime
View raw message