For these asserts, yes. It's mostly there to satisfy mypy, and is as unreachable as anything can be outside of hacking on the code.
I am in favour of using asserts for this class of check which isn't needed at runtime, but is a developer ”aid”. Also running python with -O is rarely done, because modules outside of our control do get this wrong. -ash On 3 December 2019 20:10:12 GMT, "Shaw, Damian P. " <[email protected]> wrote: >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 >=============================================================================== >
