Asserts are strictly a developer tool, and as a Airflow cluster operator I would _really_ want to know if something happens that "should never happen in reality" since those are the worst class of bugs. I think that almost any case we'd want to assert on, should actually be an exception. Even if we want to have "optional" exceptions, I'd rather see a framework for exception severity (turning some classes of exceptions, like consistency errors, into warning log lines instead).
On Tue, Dec 3, 2019 at 10:47 AM 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/> >
