Hi all, Semantic arguments aside about the meaning of assert, or the visual nicety of raise exception vs. assert, then there is one practical difference between assert vs. any other exception raise mechanism: the assert statement is filtered out when generating bytecode with the "-O" flag.
Given Airflow is so widely used I have to imagine *some* users of Airflow use pretty much every possible configuration of starting Python. So for these statements is the desired effect they are filtered from byte code when a user uses the "-O" flag? Or asked differently, given the "-O" flag would typically be used in a production environment to optimize code, is the intention that if a user looking to optimize their code in production then these statements should never run? >From reading the code I assume the answer is no, but I may be missing >something. Regards Damian -----Original Message----- From: Bjorn Olsen <[email protected]> Sent: Tuesday, December 3, 2019 2:54 PM To: [email protected] Subject: Re: [DISCUSS] Using asserts in airflow code Hi all, This SO answer helped me on another project: https://stackoverflow.com/a/41721518/2409299 The goal of an assertion in Python is to inform developers about *unrecoverable* errors in a program. Assertions are not intended to signal expected error conditions, like “file not found”, where a user can take corrective action (or just try again). My opinion - it's simpler to avoid asserts and rather use Exceptions. The CPU consumed to report an Exception is negligible compared to developer time trying to figure out an assert. If an assert is optimized out and the condition causing it does get triggered, then this can cause unexpected and untested behavior in the program if it continues past where the assert was. An Exception isn't optimised out and therefore will trigger as expected. Perhaps a new kind of Exception - UnrecoverableException - would be best. However if we do decide to use asserts then it should be used sparingly, and only for specific conditions such as above. Lastly, consider that it is easy to write an assert which doesn't work as expected: Always evaluates True: assert (2 + 2 == 5, "Houston we've got a problem") Always evaluates False: assert 2 + 2 == 5, "Houston we've got a problem" Kind regards On Tue, Dec 3, 2019 at 7:06 PM Iuliia Volkova <[email protected]> wrote: > So, now, in this code I will have no idea that is this, is this error > from Airflow or somebody forget to remove from master code debug > assert? So with normal error it will be like this: > > *assert self.futures, NOT_STARTED_MESSAGE* > > *if not self.futures: * > * raise AirflowException(NOT_STARTED_MESSAGE)* > > second variant: more readable, does not cause any issues with any > flags, I see in traceback what kind of error is this - some random > Apache Airflow or maybe ValueError, or maybe TypeError - I have more > information as developer > > And at the end of the day '*debug*' tool not used in production code. > > > On Tue, Dec 3, 2019 at 7:53 PM 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-4c0c36f193f2cd6 > 5e2b55ba3102c1ba2R38 > > 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-state > ment > > > > > > > > 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/p157536466404130 > > 0 > > > > > > > > > > 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-14337662 > 9 > > > > > > > > > > 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/> > > > > > -- > _________ > > С уважением, Юлия Волкова > Тел. : +7 (911) 116-71-82 > =============================================================================== Please access the attached hyperlink for an important electronic communications disclaimer: http://www.credit-suisse.com/legal/en/disclaimer_email_ib.html ===============================================================================
