-1 from me for using asserts in Airflow code. Firstly, as some folks pointed out, asserts are there mainly for debugging purposes.
Second, even though using asserts may bring some benefits in specific context, it’s not enough to “break even” comparing with the potential confusions (e.g. blur boundary between when to use and when NOT to use asserts over raising exceptions). XD On Wed, Dec 4, 2019 at 06:17 Anton Zayniev <[email protected]> wrote: > Email is not so good for code examples, so I've shared my thoughts here > <https://github.com/apache/airflow/pull/6596/files#r353451761>. > > On Tue, 3 Dec 2019 at 16:53, Jarek Potiuk <[email protected]> > wrote: > > > Just an example of such asserts which IMHO are nicer are here: > > > > > https://github.com/apache/airflow/pull/6596/files#diff-4c0c36f193f2cd65e2b55ba3102c1ba2R38 > > One line assert with message. > > > > J. > > > > > > > > On Tue, Dec 3, 2019 at 5:36 PM Anton Zayniev <[email protected]> > > wrote: > > > > > Hi, guys. I'm really surprised about this > > > > > > > - (+) asserts look nicer and are more readable than if (something): > > > > throw Exception() > > > > > > I'm pretty sure that all the code I have encountered a way more > readable > > > using "if/else" or "try/except". But may be it is just me. Could you > > > provide an example of code which is better with "assert"? > > > > > > - (+) asserts are especially good for cases like None exception - they > > > > add more developer friendly messages when they will fail a few > lines > > > > below > > > > with (for example) None has no property "dag". But it's ok if > those > > > get > > > > optimised away. > > > > > > I think the best way to catch None is to ensure your code would fail > > > conveniently. Like raising understandable Exception message, if you > > believe > > > that should be a point of confusion. > > > > > > On Tue, 3 Dec 2019 at 16:22, Iuliia Volkova <[email protected]> > wrote: > > > > > > > Hi everyone, I'm usually not write anything in this mail list, but > this > > > > theme something really strange > > > > Exist offissial doc: > > > > > > > > > > https://docs.python.org/3/reference/simple_stmts.html#the-assert-statement > > > > > > > > and there is a key information: Assert statements are a convenient > way > > to > > > > insert debugging assertions into a program. > > > > > > > > *Debugging. * - this is a key propose of asserts keyword. > > > > > > > > there is no any type of possible asserts that cannot be done with > > normal > > > > Exceptions and Errors types that more explicit and detailed when > > > 'assert' - > > > > you have ValueError, TyperError and etc. what kind of problems must > > > solved > > > > DEBUG tools in production code that can be easily turned off on > servers > > > by > > > > users? > > > > > > > > asserts used in tests and in process of debug code, not in production > > > > > > > > On Tue, Dec 3, 2019 at 6:47 PM Jarek Potiuk < > [email protected]> > > > > wrote: > > > > > > > > > We had a few discussions about using asserts in our code. I pasted > > some > > > > > links below but wanted to extract a gist of it. > > > > > > > > > > Here are the comments summarised: > > > > > > > > > > - (+) asserts look nicer and are more readable than if > > (something): > > > > > throw Exception() > > > > > - (-) asserts can be optimized away with -O flag so we should > not > > > > based > > > > > any real logic on having them > > > > > - (+) asserts are good in cases that can happen in development > but > > > > > should "never happen" in reality > > > > > - (+) asserts are especially good for cases like None exception > - > > > they > > > > > add more developer friendly messages when they will fail a few > > lines > > > > > below > > > > > with (for example) None has no property "dag". But it's ok if > > those > > > > get > > > > > optimised away. > > > > > > > > > > We would like to discuss those points in community and have a > > > community - > > > > > driven decision on: > > > > > > > > > > 1) whether we should use asserts? > > > > > 2) in which cases > > > > > 3) in which cases we should NOT use asserts. > > > > > > > > > > J. > > > > > > > > > > The links here: > > > > > > > > > > Slack Discussion: > > > > > > > https://apache-airflow.slack.com/archives/CCQ7EGB1P/p1575364664041300 > > > > > > > > > > Github threads: > > > > > > > > > > - > > https://github.com/apache/airflow/pull/6596#discussion_r352916409 > > > > > - > > https://github.com/apache/airflow/pull/6596#discussion_r352914727 > > > > > - > > > > > > > > > https://github.com/apache/airflow/pull/3690#pullrequestreview-143376629 > > > > > > > > > > Stack overflow link for asserts: > > > > > > > > > > - https://stackoverflow.com/a/1838411/5691525 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > J. > > > > > > > > > > -- > > > > > > > > > > Jarek Potiuk > > > > > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > > > > > > > > M: +48 660 796 129 <+48660796129> > > > > > [image: Polidea] <https://www.polidea.com/> > > > > > > > > > > > > > > > > > -- > > > > _________ > > > > > > > > С уважением, Юлия Волкова > > > > Тел. : +7 (911) 116-71-82 > > > > > > > > > > > > > -- > > > > Jarek Potiuk > > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > > M: +48 660 796 129 <+48660796129> > > [image: Polidea] <https://www.polidea.com/> > > >
