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 <philgagn...@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 <jarek.pot...@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/>
