If I added a warning in the method that it calls immediately, IDE still could not detect everything correctly. The user is not able to see the error message in the IDE. The warning appears in the wrong place. https://i.imgur.com/CSqP1wU.png
Adding a new import allows you to avoid some IDE warnings, but the problem remains that this module is not detected as deprecated https://i.imgur.com/CSqP1wU.png . On Thu, Jun 6, 2019 at 11:32 AM Ash Berlin-Taylor <a...@apache.org> wrote: > > 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.breg...@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 <a...@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.pot...@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.breg...@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.pot...@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.breg...@polidea.com> > >>>>> wrote: > >>>>> > >>>>>> Inline comment > >>>>>> > >>>>>> On Fri, Apr 19, 2019 at 12:23 AM Arthur Wiedmer < > >>>>> arthur.wied...@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.breg...@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.pot...@polidea.com > >>>>> > >>>> > >>>> > >>>> -- > >>>> > >>>> Kamil Breguła > >>>> Polidea <https://www.polidea.com/> | Software Engineer > >>>> > >>>> M: +48 505 458 451 <+48505458451> > >>>> E: kamil.breg...@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.pot...@polidea.com > >> > >> > > > > -- > > > > Kamil Breguła > > Polidea <https://www.polidea.com/> | Software Engineer > > > > M: +48 505 458 451 <+48505458451> > > E: kamil.breg...@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> >
