You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Jarek Potiuk <Ja...@polidea.com> on 2019/12/05 14:56:24 UTC
[VOTE] Allow using asserts in Airflow code
Here is a quick vote on using asserts in Airflow code.
It is distilled from the discussion
https://lists.apache.org/list.html?dev@airflow.apache.org.
Here are the two options:
*[+1]* Allow using asserts in some specific cases.*
*[-1]**: Forbid using asserts.*
The voting will last till Monday 4 pm CET. The committers have binding
votes, but everyone is encouraged to call advisory - non-binding - votes as
well.
Consider that my +1 (binding) vote.
* [+1] The case are clearly "strictly meant for developers" assertions
(None fields mainly) - which are more like type annotations and can be
stripped away when optimising. If those asserts are stripped out, another
exception will be thrown out shortly. If we agree to that we will add some
clear rules for those asserts in CONTRIBUTING.md and make it part of code
review process to check if assertions are "proper".
** [-1] Forbidding using asserts is mainly due to ambiguities when to
use/when to not use asserts. If we agree to that, we will forbid using
asserts via pre-commits and remove all assertions in our code.
J.
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>
Re: [VOTE] Allow using asserts in Airflow code
Posted by Jarek Potiuk <Ja...@polidea.com>.
Hello everyone,
It's 3 minutes to go, but I do not expect changes :).
Here are the voting results:
*[+1] Allow using asserts in some specific cases.* : 2 binding votes (Ash,
Jarek)
*[-1] Forbid using asserts.* : 6 binding votes (Kamil - yes Kamil you
have a binding vote :) , Tao, Felix, Kevin, Deng, Kaxil), 2 non-binding
votes (Philippe, Tomasz).
The voting result is *"-1": Forbid using asserts*. Anticipating the result
of the vote I prepared a PR to address it:
https://github.com/apache/airflow/pull/6749. It's already reviewed and
approved so I merge it now.
BTW. That's the first time we use our own, very simple custom Pylint plugin
(It is inspired by Bas' awesome https://github.com/BasPH/pylint-airflow ) -
we should use more of those in the future. I think we should also endorse
pylint-airflow officially as a way to improve quality of DAGs written by
everyone.
J.
On Sat, Dec 7, 2019 at 4:19 PM Philippe Gagnon <ph...@gmail.com>
wrote:
> -1 (non-binding).
>
> I can see where you are coming from, but in my opinion checks in the
> codebase should be used to direct runtime control flow. Otherwise I think
> they belong in proper tests.
>
> On Thu, Dec 5, 2019 at 9:56 AM Jarek Potiuk <Ja...@polidea.com>
> wrote:
>
> > Here is a quick vote on using asserts in Airflow code.
> >
> > It is distilled from the discussion
> > https://lists.apache.org/list.html?dev@airflow.apache.org.
> >
> > Here are the two options:
> >
> > *[+1]* Allow using asserts in some specific cases.*
> > *[-1]**: Forbid using asserts.*
> >
> > The voting will last till Monday 4 pm CET. The committers have binding
> > votes, but everyone is encouraged to call advisory - non-binding - votes
> as
> > well.
> >
> > Consider that my +1 (binding) vote.
> >
> >
> > * [+1] The case are clearly "strictly meant for developers" assertions
> > (None fields mainly) - which are more like type annotations and can be
> > stripped away when optimising. If those asserts are stripped out, another
> > exception will be thrown out shortly. If we agree to that we will add
> some
> > clear rules for those asserts in CONTRIBUTING.md and make it part of
> code
> > review process to check if assertions are "proper".
> >
> > ** [-1] Forbidding using asserts is mainly due to ambiguities when to
> > use/when to not use asserts. If we agree to that, we will forbid using
> > asserts via pre-commits and remove all assertions in our code.
> >
> > J.
> > --
> >
> > 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/>
Re: [VOTE] Allow using asserts in Airflow code
Posted by Philippe Gagnon <ph...@gmail.com>.
-1 (non-binding).
I can see where you are coming from, but in my opinion checks in the
codebase should be used to direct runtime control flow. Otherwise I think
they belong in proper tests.
On Thu, Dec 5, 2019 at 9:56 AM Jarek Potiuk <Ja...@polidea.com>
wrote:
> Here is a quick vote on using asserts in Airflow code.
>
> It is distilled from the discussion
> https://lists.apache.org/list.html?dev@airflow.apache.org.
>
> Here are the two options:
>
> *[+1]* Allow using asserts in some specific cases.*
> *[-1]**: Forbid using asserts.*
>
> The voting will last till Monday 4 pm CET. The committers have binding
> votes, but everyone is encouraged to call advisory - non-binding - votes as
> well.
>
> Consider that my +1 (binding) vote.
>
>
> * [+1] The case are clearly "strictly meant for developers" assertions
> (None fields mainly) - which are more like type annotations and can be
> stripped away when optimising. If those asserts are stripped out, another
> exception will be thrown out shortly. If we agree to that we will add some
> clear rules for those asserts in CONTRIBUTING.md and make it part of code
> review process to check if assertions are "proper".
>
> ** [-1] Forbidding using asserts is mainly due to ambiguities when to
> use/when to not use asserts. If we agree to that, we will forbid using
> asserts via pre-commits and remove all assertions in our code.
>
> J.
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
>
Re: [VOTE] Allow using asserts in Airflow code
Posted by Kamil Breguła <ka...@polidea.com>.
-1 (non-binding) This can cause too much confusion for new contributor.
On Fri, Dec 6, 2019 at 12:05 PM Ash Berlin-Taylor <as...@apache.org> wrote:
> +1 (binding) as I've already said :)
>
> -ash
>
> > On 6 Dec 2019, at 00:55, Tao Feng <fe...@gmail.com> wrote:
> >
> > -1 (binding)
> >
> > I share the same with most other comments. And I personally prefer to use
> > try,except to make it consistent across the code base while use assert in
> > unit test .
> >
> > On Thu, Dec 5, 2019 at 3:09 PM Felix Uellendall <fe...@pm.me.invalid>
> > wrote:
> >
> >> -1 (binding)
> >>
> >> I agree. There shouldn’t be any confusion around this if we want to
> >> introduce this. The old/current assertion style still looks more
> readable
> >> to me.
> >>
> >> Felix
> >>
> >> Sent from ProtonMail Mobile
> >>
> >> On Thu, Dec 5, 2019 at 23:35, Kevin Yang <yr...@gmail.com> wrote:
> >>
> >>> -1 (binding).
> >>>
> >>> People in the old thread has spoken for me. Specifically in Python, the
> >>> confusion introduced by using asserts IMO can defeat all the benefits
> >>> mentioned easily.
> >>>
> >>> Kevin Y
> >>>
> >>> On Thu, Dec 5, 2019 at 8:27 AM Tomasz Urbaszek <
> >> tomasz.urbaszek@polidea.com>
> >>> wrote:
> >>>
> >>>> -1 (non-binding)
> >>>>
> >>>> T.
> >>>>
> >>>>
> >>>> On Thu, Dec 5, 2019 at 4:16 PM Deng Xiaodong <xd...@gmail.com>
> >> wrote:
> >>>>
> >>>>> -1 (binding).
> >>>>>
> >>>>> As shared earlier, the benefit it brings may not be enough to break
> >> even
> >>>>> for me. And it’s not irreplaceable.
> >>>>>
> >>>>>
> >>>>> XD
> >>>>>
> >>>>>> On 5 Dec 2019, at 11:10 PM, Kaxil Naik <ka...@gmail.com> wrote:
> >>>>>>
> >>>>>> -1 (binding) it definitely seems to be a source of confusion and
> >>>>> comparing
> >>>>>> it to the advantages it provides, I would be hesitant on using it.
> >>>>>>
> >>>>>> On Thu, Dec 5, 2019 at 2:56 PM Jarek Potiuk <
> >> Jarek.Potiuk@polidea.com>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> Here is a quick vote on using asserts in Airflow code.
> >>>>>>>
> >>>>>>> It is distilled from the discussion
> >>>>>>> https://lists.apache.org/list.html?dev@airflow.apache.org.
> >>>>>>>
> >>>>>>> Here are the two options:
> >>>>>>>
> >>>>>>> *[+1]* Allow using asserts in some specific cases.*
> >>>>>>> *[-1]**: Forbid using asserts.*
> >>>>>>>
> >>>>>>> The voting will last till Monday 4 pm CET. The committers have
> >> binding
> >>>>>>> votes, but everyone is encouraged to call advisory - non-binding -
> >>>>> votes as
> >>>>>>> well.
> >>>>>>>
> >>>>>>> Consider that my +1 (binding) vote.
> >>>>>>>
> >>>>>>>
> >>>>>>> * [+1] The case are clearly "strictly meant for developers"
> >> assertions
> >>>>>>> (None fields mainly) - which are more like type annotations and
> >> can be
> >>>>>>> stripped away when optimising. If those asserts are stripped out,
> >>>>> another
> >>>>>>> exception will be thrown out shortly. If we agree to that we will
> >> add
> >>>>> some
> >>>>>>> clear rules for those asserts in CONTRIBUTING.md and make it part
> >> of
> >>>>> code
> >>>>>>> review process to check if assertions are "proper".
> >>>>>>>
> >>>>>>> ** [-1] Forbidding using asserts is mainly due to ambiguities when
> >> to
> >>>>>>> use/when to not use asserts. If we agree to that, we will forbid
> >> using
> >>>>>>> asserts via pre-commits and remove all assertions in our code.
> >>>>>>>
> >>>>>>> J.
> >>>>>>> --
> >>>>>>>
> >>>>>>> Jarek Potiuk
> >>>>>>> Polidea <https://www.polidea.com/> | Principal Software Engineer
> >>>>>>>
> >>>>>>> M: +48 660 796 129 <+48660796129>
> >>>>>>> [image: Polidea] <https://www.polidea.com/>
> >>>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>> --
> >>>>
> >>>> Tomasz Urbaszek
> >>>> Polidea <https://www.polidea.com/> | Junior Software Engineer
> >>>>
> >>>> M: +48 505 628 493 <+48505628493>
> >>>> E: tomasz.urbaszek@polidea.com <to...@polidea.com>
> >>>>
> >>>> Unique Tech
> >>>> Check out our projects! <https://www.polidea.com/our-work>
> >>>>
>
>
Re: [VOTE] Allow using asserts in Airflow code
Posted by Ash Berlin-Taylor <as...@apache.org>.
+1 (binding) as I've already said :)
-ash
> On 6 Dec 2019, at 00:55, Tao Feng <fe...@gmail.com> wrote:
>
> -1 (binding)
>
> I share the same with most other comments. And I personally prefer to use
> try,except to make it consistent across the code base while use assert in
> unit test .
>
> On Thu, Dec 5, 2019 at 3:09 PM Felix Uellendall <fe...@pm.me.invalid>
> wrote:
>
>> -1 (binding)
>>
>> I agree. There shouldn’t be any confusion around this if we want to
>> introduce this. The old/current assertion style still looks more readable
>> to me.
>>
>> Felix
>>
>> Sent from ProtonMail Mobile
>>
>> On Thu, Dec 5, 2019 at 23:35, Kevin Yang <yr...@gmail.com> wrote:
>>
>>> -1 (binding).
>>>
>>> People in the old thread has spoken for me. Specifically in Python, the
>>> confusion introduced by using asserts IMO can defeat all the benefits
>>> mentioned easily.
>>>
>>> Kevin Y
>>>
>>> On Thu, Dec 5, 2019 at 8:27 AM Tomasz Urbaszek <
>> tomasz.urbaszek@polidea.com>
>>> wrote:
>>>
>>>> -1 (non-binding)
>>>>
>>>> T.
>>>>
>>>>
>>>> On Thu, Dec 5, 2019 at 4:16 PM Deng Xiaodong <xd...@gmail.com>
>> wrote:
>>>>
>>>>> -1 (binding).
>>>>>
>>>>> As shared earlier, the benefit it brings may not be enough to break
>> even
>>>>> for me. And it’s not irreplaceable.
>>>>>
>>>>>
>>>>> XD
>>>>>
>>>>>> On 5 Dec 2019, at 11:10 PM, Kaxil Naik <ka...@gmail.com> wrote:
>>>>>>
>>>>>> -1 (binding) it definitely seems to be a source of confusion and
>>>>> comparing
>>>>>> it to the advantages it provides, I would be hesitant on using it.
>>>>>>
>>>>>> On Thu, Dec 5, 2019 at 2:56 PM Jarek Potiuk <
>> Jarek.Potiuk@polidea.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Here is a quick vote on using asserts in Airflow code.
>>>>>>>
>>>>>>> It is distilled from the discussion
>>>>>>> https://lists.apache.org/list.html?dev@airflow.apache.org.
>>>>>>>
>>>>>>> Here are the two options:
>>>>>>>
>>>>>>> *[+1]* Allow using asserts in some specific cases.*
>>>>>>> *[-1]**: Forbid using asserts.*
>>>>>>>
>>>>>>> The voting will last till Monday 4 pm CET. The committers have
>> binding
>>>>>>> votes, but everyone is encouraged to call advisory - non-binding -
>>>>> votes as
>>>>>>> well.
>>>>>>>
>>>>>>> Consider that my +1 (binding) vote.
>>>>>>>
>>>>>>>
>>>>>>> * [+1] The case are clearly "strictly meant for developers"
>> assertions
>>>>>>> (None fields mainly) - which are more like type annotations and
>> can be
>>>>>>> stripped away when optimising. If those asserts are stripped out,
>>>>> another
>>>>>>> exception will be thrown out shortly. If we agree to that we will
>> add
>>>>> some
>>>>>>> clear rules for those asserts in CONTRIBUTING.md and make it part
>> of
>>>>> code
>>>>>>> review process to check if assertions are "proper".
>>>>>>>
>>>>>>> ** [-1] Forbidding using asserts is mainly due to ambiguities when
>> to
>>>>>>> use/when to not use asserts. If we agree to that, we will forbid
>> using
>>>>>>> asserts via pre-commits and remove all assertions in our code.
>>>>>>>
>>>>>>> J.
>>>>>>> --
>>>>>>>
>>>>>>> Jarek Potiuk
>>>>>>> Polidea <https://www.polidea.com/> | Principal Software Engineer
>>>>>>>
>>>>>>> M: +48 660 796 129 <+48660796129>
>>>>>>> [image: Polidea] <https://www.polidea.com/>
>>>>>>>
>>>>>
>>>>>
>>>>
>>>> --
>>>>
>>>> Tomasz Urbaszek
>>>> Polidea <https://www.polidea.com/> | Junior Software Engineer
>>>>
>>>> M: +48 505 628 493 <+48505628493>
>>>> E: tomasz.urbaszek@polidea.com <to...@polidea.com>
>>>>
>>>> Unique Tech
>>>> Check out our projects! <https://www.polidea.com/our-work>
>>>>
Re: [VOTE] Allow using asserts in Airflow code
Posted by Tao Feng <fe...@gmail.com>.
-1 (binding)
I share the same with most other comments. And I personally prefer to use
try,except to make it consistent across the code base while use assert in
unit test .
On Thu, Dec 5, 2019 at 3:09 PM Felix Uellendall <fe...@pm.me.invalid>
wrote:
> -1 (binding)
>
> I agree. There shouldn’t be any confusion around this if we want to
> introduce this. The old/current assertion style still looks more readable
> to me.
>
> Felix
>
> Sent from ProtonMail Mobile
>
> On Thu, Dec 5, 2019 at 23:35, Kevin Yang <yr...@gmail.com> wrote:
>
> > -1 (binding).
> >
> > People in the old thread has spoken for me. Specifically in Python, the
> > confusion introduced by using asserts IMO can defeat all the benefits
> > mentioned easily.
> >
> > Kevin Y
> >
> > On Thu, Dec 5, 2019 at 8:27 AM Tomasz Urbaszek <
> tomasz.urbaszek@polidea.com>
> > wrote:
> >
> >> -1 (non-binding)
> >>
> >> T.
> >>
> >>
> >> On Thu, Dec 5, 2019 at 4:16 PM Deng Xiaodong <xd...@gmail.com>
> wrote:
> >>
> >> > -1 (binding).
> >> >
> >> > As shared earlier, the benefit it brings may not be enough to break
> even
> >> > for me. And it’s not irreplaceable.
> >> >
> >> >
> >> > XD
> >> >
> >> > > On 5 Dec 2019, at 11:10 PM, Kaxil Naik <ka...@gmail.com> wrote:
> >> > >
> >> > > -1 (binding) it definitely seems to be a source of confusion and
> >> > comparing
> >> > > it to the advantages it provides, I would be hesitant on using it.
> >> > >
> >> > > On Thu, Dec 5, 2019 at 2:56 PM Jarek Potiuk <
> Jarek.Potiuk@polidea.com>
> >> > > wrote:
> >> > >
> >> > >> Here is a quick vote on using asserts in Airflow code.
> >> > >>
> >> > >> It is distilled from the discussion
> >> > >> https://lists.apache.org/list.html?dev@airflow.apache.org.
> >> > >>
> >> > >> Here are the two options:
> >> > >>
> >> > >> *[+1]* Allow using asserts in some specific cases.*
> >> > >> *[-1]**: Forbid using asserts.*
> >> > >>
> >> > >> The voting will last till Monday 4 pm CET. The committers have
> binding
> >> > >> votes, but everyone is encouraged to call advisory - non-binding -
> >> > votes as
> >> > >> well.
> >> > >>
> >> > >> Consider that my +1 (binding) vote.
> >> > >>
> >> > >>
> >> > >> * [+1] The case are clearly "strictly meant for developers"
> assertions
> >> > >> (None fields mainly) - which are more like type annotations and
> can be
> >> > >> stripped away when optimising. If those asserts are stripped out,
> >> > another
> >> > >> exception will be thrown out shortly. If we agree to that we will
> add
> >> > some
> >> > >> clear rules for those asserts in CONTRIBUTING.md and make it part
> of
> >> > code
> >> > >> review process to check if assertions are "proper".
> >> > >>
> >> > >> ** [-1] Forbidding using asserts is mainly due to ambiguities when
> to
> >> > >> use/when to not use asserts. If we agree to that, we will forbid
> using
> >> > >> asserts via pre-commits and remove all assertions in our code.
> >> > >>
> >> > >> J.
> >> > >> --
> >> > >>
> >> > >> Jarek Potiuk
> >> > >> Polidea <https://www.polidea.com/> | Principal Software Engineer
> >> > >>
> >> > >> M: +48 660 796 129 <+48660796129>
> >> > >> [image: Polidea] <https://www.polidea.com/>
> >> > >>
> >> >
> >> >
> >>
> >> --
> >>
> >> Tomasz Urbaszek
> >> Polidea <https://www.polidea.com/> | Junior Software Engineer
> >>
> >> M: +48 505 628 493 <+48505628493>
> >> E: tomasz.urbaszek@polidea.com <to...@polidea.com>
> >>
> >> Unique Tech
> >> Check out our projects! <https://www.polidea.com/our-work>
> >>
Re: [VOTE] Allow using asserts in Airflow code
Posted by Felix Uellendall <fe...@pm.me.INVALID>.
-1 (binding)
I agree. There shouldn’t be any confusion around this if we want to introduce this. The old/current assertion style still looks more readable to me.
Felix
Sent from ProtonMail Mobile
On Thu, Dec 5, 2019 at 23:35, Kevin Yang <yr...@gmail.com> wrote:
> -1 (binding).
>
> People in the old thread has spoken for me. Specifically in Python, the
> confusion introduced by using asserts IMO can defeat all the benefits
> mentioned easily.
>
> Kevin Y
>
> On Thu, Dec 5, 2019 at 8:27 AM Tomasz Urbaszek <to...@polidea.com>
> wrote:
>
>> -1 (non-binding)
>>
>> T.
>>
>>
>> On Thu, Dec 5, 2019 at 4:16 PM Deng Xiaodong <xd...@gmail.com> wrote:
>>
>> > -1 (binding).
>> >
>> > As shared earlier, the benefit it brings may not be enough to break even
>> > for me. And it’s not irreplaceable.
>> >
>> >
>> > XD
>> >
>> > > On 5 Dec 2019, at 11:10 PM, Kaxil Naik <ka...@gmail.com> wrote:
>> > >
>> > > -1 (binding) it definitely seems to be a source of confusion and
>> > comparing
>> > > it to the advantages it provides, I would be hesitant on using it.
>> > >
>> > > On Thu, Dec 5, 2019 at 2:56 PM Jarek Potiuk <Ja...@polidea.com>
>> > > wrote:
>> > >
>> > >> Here is a quick vote on using asserts in Airflow code.
>> > >>
>> > >> It is distilled from the discussion
>> > >> https://lists.apache.org/list.html?dev@airflow.apache.org.
>> > >>
>> > >> Here are the two options:
>> > >>
>> > >> *[+1]* Allow using asserts in some specific cases.*
>> > >> *[-1]**: Forbid using asserts.*
>> > >>
>> > >> The voting will last till Monday 4 pm CET. The committers have binding
>> > >> votes, but everyone is encouraged to call advisory - non-binding -
>> > votes as
>> > >> well.
>> > >>
>> > >> Consider that my +1 (binding) vote.
>> > >>
>> > >>
>> > >> * [+1] The case are clearly "strictly meant for developers" assertions
>> > >> (None fields mainly) - which are more like type annotations and can be
>> > >> stripped away when optimising. If those asserts are stripped out,
>> > another
>> > >> exception will be thrown out shortly. If we agree to that we will add
>> > some
>> > >> clear rules for those asserts in CONTRIBUTING.md and make it part of
>> > code
>> > >> review process to check if assertions are "proper".
>> > >>
>> > >> ** [-1] Forbidding using asserts is mainly due to ambiguities when to
>> > >> use/when to not use asserts. If we agree to that, we will forbid using
>> > >> asserts via pre-commits and remove all assertions in our code.
>> > >>
>> > >> J.
>> > >> --
>> > >>
>> > >> Jarek Potiuk
>> > >> Polidea <https://www.polidea.com/> | Principal Software Engineer
>> > >>
>> > >> M: +48 660 796 129 <+48660796129>
>> > >> [image: Polidea] <https://www.polidea.com/>
>> > >>
>> >
>> >
>>
>> --
>>
>> Tomasz Urbaszek
>> Polidea <https://www.polidea.com/> | Junior Software Engineer
>>
>> M: +48 505 628 493 <+48505628493>
>> E: tomasz.urbaszek@polidea.com <to...@polidea.com>
>>
>> Unique Tech
>> Check out our projects! <https://www.polidea.com/our-work>
>>
Re: [VOTE] Allow using asserts in Airflow code
Posted by Kevin Yang <yr...@gmail.com>.
-1 (binding).
People in the old thread has spoken for me. Specifically in Python, the
confusion introduced by using asserts IMO can defeat all the benefits
mentioned easily.
Kevin Y
On Thu, Dec 5, 2019 at 8:27 AM Tomasz Urbaszek <to...@polidea.com>
wrote:
> -1 (non-binding)
>
> T.
>
>
> On Thu, Dec 5, 2019 at 4:16 PM Deng Xiaodong <xd...@gmail.com> wrote:
>
> > -1 (binding).
> >
> > As shared earlier, the benefit it brings may not be enough to break even
> > for me. And it’s not irreplaceable.
> >
> >
> > XD
> >
> > > On 5 Dec 2019, at 11:10 PM, Kaxil Naik <ka...@gmail.com> wrote:
> > >
> > > -1 (binding) it definitely seems to be a source of confusion and
> > comparing
> > > it to the advantages it provides, I would be hesitant on using it.
> > >
> > > On Thu, Dec 5, 2019 at 2:56 PM Jarek Potiuk <Ja...@polidea.com>
> > > wrote:
> > >
> > >> Here is a quick vote on using asserts in Airflow code.
> > >>
> > >> It is distilled from the discussion
> > >> https://lists.apache.org/list.html?dev@airflow.apache.org.
> > >>
> > >> Here are the two options:
> > >>
> > >> *[+1]* Allow using asserts in some specific cases.*
> > >> *[-1]**: Forbid using asserts.*
> > >>
> > >> The voting will last till Monday 4 pm CET. The committers have binding
> > >> votes, but everyone is encouraged to call advisory - non-binding -
> > votes as
> > >> well.
> > >>
> > >> Consider that my +1 (binding) vote.
> > >>
> > >>
> > >> * [+1] The case are clearly "strictly meant for developers" assertions
> > >> (None fields mainly) - which are more like type annotations and can be
> > >> stripped away when optimising. If those asserts are stripped out,
> > another
> > >> exception will be thrown out shortly. If we agree to that we will add
> > some
> > >> clear rules for those asserts in CONTRIBUTING.md and make it part of
> > code
> > >> review process to check if assertions are "proper".
> > >>
> > >> ** [-1] Forbidding using asserts is mainly due to ambiguities when to
> > >> use/when to not use asserts. If we agree to that, we will forbid using
> > >> asserts via pre-commits and remove all assertions in our code.
> > >>
> > >> J.
> > >> --
> > >>
> > >> Jarek Potiuk
> > >> Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >>
> > >> M: +48 660 796 129 <+48660796129>
> > >> [image: Polidea] <https://www.polidea.com/>
> > >>
> >
> >
>
> --
>
> Tomasz Urbaszek
> Polidea <https://www.polidea.com/> | Junior Software Engineer
>
> M: +48 505 628 493 <+48505628493>
> E: tomasz.urbaszek@polidea.com <to...@polidea.com>
>
> Unique Tech
> Check out our projects! <https://www.polidea.com/our-work>
>
Re: [VOTE] Allow using asserts in Airflow code
Posted by Tomasz Urbaszek <to...@polidea.com>.
-1 (non-binding)
T.
On Thu, Dec 5, 2019 at 4:16 PM Deng Xiaodong <xd...@gmail.com> wrote:
> -1 (binding).
>
> As shared earlier, the benefit it brings may not be enough to break even
> for me. And it’s not irreplaceable.
>
>
> XD
>
> > On 5 Dec 2019, at 11:10 PM, Kaxil Naik <ka...@gmail.com> wrote:
> >
> > -1 (binding) it definitely seems to be a source of confusion and
> comparing
> > it to the advantages it provides, I would be hesitant on using it.
> >
> > On Thu, Dec 5, 2019 at 2:56 PM Jarek Potiuk <Ja...@polidea.com>
> > wrote:
> >
> >> Here is a quick vote on using asserts in Airflow code.
> >>
> >> It is distilled from the discussion
> >> https://lists.apache.org/list.html?dev@airflow.apache.org.
> >>
> >> Here are the two options:
> >>
> >> *[+1]* Allow using asserts in some specific cases.*
> >> *[-1]**: Forbid using asserts.*
> >>
> >> The voting will last till Monday 4 pm CET. The committers have binding
> >> votes, but everyone is encouraged to call advisory - non-binding -
> votes as
> >> well.
> >>
> >> Consider that my +1 (binding) vote.
> >>
> >>
> >> * [+1] The case are clearly "strictly meant for developers" assertions
> >> (None fields mainly) - which are more like type annotations and can be
> >> stripped away when optimising. If those asserts are stripped out,
> another
> >> exception will be thrown out shortly. If we agree to that we will add
> some
> >> clear rules for those asserts in CONTRIBUTING.md and make it part of
> code
> >> review process to check if assertions are "proper".
> >>
> >> ** [-1] Forbidding using asserts is mainly due to ambiguities when to
> >> use/when to not use asserts. If we agree to that, we will forbid using
> >> asserts via pre-commits and remove all assertions in our code.
> >>
> >> J.
> >> --
> >>
> >> Jarek Potiuk
> >> Polidea <https://www.polidea.com/> | Principal Software Engineer
> >>
> >> M: +48 660 796 129 <+48660796129>
> >> [image: Polidea] <https://www.polidea.com/>
> >>
>
>
--
Tomasz Urbaszek
Polidea <https://www.polidea.com/> | Junior Software Engineer
M: +48 505 628 493 <+48505628493>
E: tomasz.urbaszek@polidea.com <to...@polidea.com>
Unique Tech
Check out our projects! <https://www.polidea.com/our-work>
Re: [VOTE] Allow using asserts in Airflow code
Posted by Deng Xiaodong <xd...@gmail.com>.
-1 (binding).
As shared earlier, the benefit it brings may not be enough to break even for me. And it’s not irreplaceable.
XD
> On 5 Dec 2019, at 11:10 PM, Kaxil Naik <ka...@gmail.com> wrote:
>
> -1 (binding) it definitely seems to be a source of confusion and comparing
> it to the advantages it provides, I would be hesitant on using it.
>
> On Thu, Dec 5, 2019 at 2:56 PM Jarek Potiuk <Ja...@polidea.com>
> wrote:
>
>> Here is a quick vote on using asserts in Airflow code.
>>
>> It is distilled from the discussion
>> https://lists.apache.org/list.html?dev@airflow.apache.org.
>>
>> Here are the two options:
>>
>> *[+1]* Allow using asserts in some specific cases.*
>> *[-1]**: Forbid using asserts.*
>>
>> The voting will last till Monday 4 pm CET. The committers have binding
>> votes, but everyone is encouraged to call advisory - non-binding - votes as
>> well.
>>
>> Consider that my +1 (binding) vote.
>>
>>
>> * [+1] The case are clearly "strictly meant for developers" assertions
>> (None fields mainly) - which are more like type annotations and can be
>> stripped away when optimising. If those asserts are stripped out, another
>> exception will be thrown out shortly. If we agree to that we will add some
>> clear rules for those asserts in CONTRIBUTING.md and make it part of code
>> review process to check if assertions are "proper".
>>
>> ** [-1] Forbidding using asserts is mainly due to ambiguities when to
>> use/when to not use asserts. If we agree to that, we will forbid using
>> asserts via pre-commits and remove all assertions in our code.
>>
>> J.
>> --
>>
>> Jarek Potiuk
>> Polidea <https://www.polidea.com/> | Principal Software Engineer
>>
>> M: +48 660 796 129 <+48660796129>
>> [image: Polidea] <https://www.polidea.com/>
>>
Re: [VOTE] Allow using asserts in Airflow code
Posted by Kaxil Naik <ka...@gmail.com>.
-1 (binding) it definitely seems to be a source of confusion and comparing
it to the advantages it provides, I would be hesitant on using it.
On Thu, Dec 5, 2019 at 2:56 PM Jarek Potiuk <Ja...@polidea.com>
wrote:
> Here is a quick vote on using asserts in Airflow code.
>
> It is distilled from the discussion
> https://lists.apache.org/list.html?dev@airflow.apache.org.
>
> Here are the two options:
>
> *[+1]* Allow using asserts in some specific cases.*
> *[-1]**: Forbid using asserts.*
>
> The voting will last till Monday 4 pm CET. The committers have binding
> votes, but everyone is encouraged to call advisory - non-binding - votes as
> well.
>
> Consider that my +1 (binding) vote.
>
>
> * [+1] The case are clearly "strictly meant for developers" assertions
> (None fields mainly) - which are more like type annotations and can be
> stripped away when optimising. If those asserts are stripped out, another
> exception will be thrown out shortly. If we agree to that we will add some
> clear rules for those asserts in CONTRIBUTING.md and make it part of code
> review process to check if assertions are "proper".
>
> ** [-1] Forbidding using asserts is mainly due to ambiguities when to
> use/when to not use asserts. If we agree to that, we will forbid using
> asserts via pre-commits and remove all assertions in our code.
>
> J.
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
>