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 Thu, 06 Jun 2019 09:32:50 GMT
I hate IDEs :(

What does the IDE show if you package up the deprecation warning in some way in your solution
(i.e. the warning.warn isn't directly in the module source)? Alternatively adding `from solution2.new
import *` into solution2.deprecated may help the IDE?

-a

> On 5 Jun 2019, at 17:41, Kamil Breguła <kamil.bregula@polidea.com> wrote:
> 
> I have done a deep investigation of this issue. Please, look at the
> beginning of chapter "Case #7".
> 
> I prepared a repository with simple demo and also shared a screenshot from
> my IDE.
> Photo: https://imgur.com/a/mRaWpQm
> Repo: https://github.com/mik-laj/airflow-deprecation-sample
> Today I looked again at this project. I recently updated the IDE to the
> latest version - IntelliJ IDEA 2019.1.2 (Ultimate Edition). The issue still
> exists.
> I made additional photos today: https://imgur.com/a/4fcmkVX
> 
> IDE uses statistical analysis of the code and gets lost in external
> libraries.
> 
> If you have questions, I will be happy to give you further explanations.
> 
> On Wed, Jun 5, 2019 at 4:30 PM Ash Berlin-Taylor <ash@apache.org> wrote:
> 
>>> 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
>> 
>> 
> 
> -- 
> 
> 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>


Mime
View raw message